Skip to content

Commit

Permalink
JUnit: migrate to v5.x (#1621)
Browse files Browse the repository at this point in the history
Ich habe mich lange für JUnit 4.x ausgesprochen, u.a. weil sich JUnit5
noch nicht reif für den Praxiseinsatz angefühlte und weil es in JUnit5
keine vernünftige Möglichkeit gab (und gibt), auf Java-Ebene Testsuiten
zu definieren (man muss dann über Gradle o.ä. gehen).

Mittlerweile ist JUnit5 aus meiner Sicht reif genug, um zu wechseln.
Testsuiten kann man zwar immer noch nur extern formulieren, aber davon
machen wir hier im Projekt keinen Gebrauch. Dafür sind die Möglichkeiten
zum Testen auf Exceptions sowie für parametrisierte Tests deutlich
überlegen im Vergleich zu JUnit4.

Let's switch over.


---

Dieser PR führt die Migration von JUnit 4.x auf JUnit 5.x durch.
Betroffen sind:

- Globale Gradle-Konfiguration: Ersetzen der bisherigen
JUnit4-Abhängigkeit durch die neue JUnit5-Abhängigkeit plus Launcher
(lt. Doku wird letzterer nur für IDEs benötigt, die eine veraltete
JUnit-Version mitbringen - brauchen wir die Launcher-Konfiguration
überhaupt?!)
- Gradle-Konfiguration für die Subprojekte "game" und "dungeon":
Hinzunahme der neuen Abhängigkeiten und Deklaration der JUnit-Plattform
für die Tests
- Testsourcen in den Subprojekten "game" und "dungeon":
    - Ersetzen der Importe
- Übersetzen von `@Before` und `@After` durch die neuen Annotationen
`@BeforeEach` und `@AfterEach`
- Übersetzen von `@BeforeClass` und `@AfterClass` durch die neuen
Annotationen `@BeforeAll` und `@AfterAll`
    -  Übersetzen von `@Ignore` durch die neue Annotatione `@Disabled`
- Übersetzen von `@Test(expected = …​)` durch den Einsatz von
`Assertions.assertThrows(...​)`
- **Anpassen der Assertions**: Es wurde an einigen Stellen
`assertEquals(string, expected, actual)` verwendet. Das gibt es so nicht
mehr bzw. der String müsste als letzter Parameter übergeben werden. Da
die verwendeten Strings in der überwiegenden Mehrzahl semantisch unklar
und nur eine Wiederholung des erwarteten Wertes waren (Beispiel:
`assertEquals("es sollte 3 rauskommen", 3, something.or.other())`), habe
ich diese Strings einfach überall entfernt. Es gab einige wenige
Stellen, wo die Strings tatsächlich eine sinnvolle semantische Aussage
enthielten. Wenn jemand Zeit hat, könnte man diese Strings nochmal
wieder einbauen.
- ~~Der **Test `contrib.entities.ChestTest.checkCreation()` schlägt auf
einmal fehl**. Wenn ich mir den Test anschaue, frage ich mich, warum der
nicht auch bereits in vorher (also in JUnit4) fehlgeschlagen ist? =>
Issue #1622~~ fixed: tausche `assertEquals` gegen die korrekte Methode
für den Use-Case: `assertArrayEquals`
(31391ea).


---

closes #1583
  • Loading branch information
cagix authored Aug 16, 2024
1 parent 0f65e4f commit 84454da
Show file tree
Hide file tree
Showing 81 changed files with 2,191 additions and 2,245 deletions.
8 changes: 5 additions & 3 deletions dependencies.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ ext {
// -- DEPENDENCIES
gdxVersion = '1.12.1'
aiVersion = '1.8.2'
junitVersion = '4.13.2'
junitVersion = '5.11.0'
junitLauncherVersion = '1.11.0'
mockitoVersion = '5.12.0'
antlrVersion = '4.13.2'

Expand All @@ -19,8 +20,9 @@ ext {
gdx_freetype : "com.badlogicgames.gdx:gdx-freetype:$gdxVersion",
gdx_freetype_platform : "com.badlogicgames.gdx:gdx-freetype-platform:$gdxVersion:natives-desktop",

// JUnit 4 and Mockito for testing
junit : "junit:junit:$junitVersion",
// JUnit and Mockito for testing
junit : "org.junit.jupiter:junit-jupiter:$junitVersion",
junitLauncher : "org.junit.platform:junit-platform-launcher:$junitLauncherVersion",
mockito_core : "org.mockito:mockito-core:$mockitoVersion",

// ANTLR version 4 for DSL Grammar
Expand Down
8 changes: 7 additions & 1 deletion dungeon/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ dependencies {
// ANTLR version 4 for DSL Grammar
antlr supportDependencies.antlr

// JUnit 4 and Mockito for testing
// JUnit and Mockito for testing
testImplementation supportDependencies.junit
testRuntimeOnly supportDependencies.junitLauncher
testImplementation supportDependencies.mockito_core
}

Expand Down Expand Up @@ -111,3 +112,8 @@ tasks.register('runYesNoDialogTest', JavaExec) {
mainClass = 'manual.YesNoDialogTest'
classpath = sourceSets.test.runtimeClasspath
}


tasks.named('test', Test) {
useJUnitPlatform()
}
36 changes: 16 additions & 20 deletions dungeon/test/contrib/components/CollisionComponentTest.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package contrib.components;

import static org.junit.Assert.*;
import static org.junit.jupiter.api.Assertions.*;

import core.Entity;
import core.components.PositionComponent;
import core.level.Tile;
import core.utils.Point;
import core.utils.TriConsumer;
import core.utils.components.MissingComponentException;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import testingUtils.SimpleCounter;

/** Collision component tests. */
Expand Down Expand Up @@ -46,10 +46,10 @@ public void onEnterCheckCall() {
e2.add(hb2);
hb1.onEnter(e1, e2, Tile.Direction.N);

assertEquals("Der Counter von Entität 1 Enter soll ", 1, counterE1Enter.getCount());
assertEquals("Der Counter von Entität 1 Leave soll ", 0, counterE1Leave.getCount());
assertEquals("Der Counter von Entität 2 Enter soll ", 0, counterE2Enter.getCount());
assertEquals("Der Counter von Entität 2 Leave soll ", 0, counterE2Leave.getCount());
assertEquals(1, counterE1Enter.getCount());
assertEquals(0, counterE1Leave.getCount());
assertEquals(0, counterE2Enter.getCount());
assertEquals(0, counterE2Leave.getCount());
}

/** On leave no method given. */
Expand Down Expand Up @@ -82,10 +82,10 @@ public void onLeaveCheckCall() {
e1.add(hb1);
e2.add(hb2);
hb1.onLeave(e1, e2, Tile.Direction.N);
assertEquals("Der Counter von Entität 1 Enter soll ", 0, counterE1Enter.getCount());
assertEquals("Der Counter von Entität 1 Leave soll ", 1, counterE1Leave.getCount());
assertEquals("Der Counter von Entität 2 Enter soll ", 0, counterE2Enter.getCount());
assertEquals("Der Counter von Entität 2 Leave soll ", 0, counterE2Leave.getCount());
assertEquals(0, counterE1Enter.getCount());
assertEquals(1, counterE1Leave.getCount());
assertEquals(0, counterE2Enter.getCount());
assertEquals(0, counterE2Leave.getCount());
}

/** WTF? . */
Expand All @@ -100,8 +100,7 @@ public void setiCollideEnterNull() {
e2.add(hb2);
hb1.collideEnter(null);
hb1.onEnter(e1, e2, Tile.Direction.N);
assertEquals(
"Die alte Collide darf nicht mehr aufgerufen werden ", 0, counterE1Enter.getCount());
assertEquals(0, counterE1Enter.getCount());
}

/** WTF? . */
Expand All @@ -117,9 +116,8 @@ public void setiCollideEnterValidCollider() {
e2.add(hb2);
hb1.collideEnter((a, b, c) -> newCounterE1Enter.inc());
hb1.onEnter(e1, e2, Tile.Direction.N);
assertEquals(
"Die alte Collide darf nicht mehr aufgerufen werden ", 0, counterE1Enter.getCount());
assertEquals("Die neue Collide muss aufgerufen werden ", 1, newCounterE1Enter.getCount());
assertEquals(0, counterE1Enter.getCount());
assertEquals(1, newCounterE1Enter.getCount());
}

/** WTF? . */
Expand All @@ -134,8 +132,7 @@ public void setiCollideLeaveNull() {
e2.add(hb2);
hb1.collideLeave(null);
hb1.onLeave(e1, e2, Tile.Direction.N);
assertEquals(
"Die alte Collide darf nicht mehr aufgerufen werden ", 0, counterE1Enter.getCount());
assertEquals(0, counterE1Enter.getCount());
}

/** WTF? . */
Expand All @@ -151,9 +148,8 @@ public void setiCollideLeaveValidCollider() {
e2.add(hb2);
hb1.collideLeave((a, b, c) -> newCounterE1Leave.inc());
hb1.onLeave(e1, e2, Tile.Direction.N);
assertEquals(
"Die alte Collide darf nicht mehr aufgerufen werden ", 0, counterE1Leave.getCount());
assertEquals("Die neue Collide muss aufgerufen werden ", 1, newCounterE1Leave.getCount());
assertEquals(0, counterE1Leave.getCount());
assertEquals(1, newCounterE1Leave.getCount());
}

/** Missing PositionComponent. */
Expand Down
4 changes: 2 additions & 2 deletions dungeon/test/contrib/components/HealthComponentTest.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
package contrib.components;

import static org.junit.Assert.assertEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.mockito.Mockito.times;

import contrib.utils.components.health.Damage;
import contrib.utils.components.health.DamageType;
import core.Entity;
import core.Game;
import java.util.function.Consumer;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

/** Tests for the {@link HealthComponent}. */
Expand Down
4 changes: 2 additions & 2 deletions dungeon/test/contrib/components/InteractionComponentTest.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package contrib.components;

import static org.junit.Assert.*;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.verify;

import core.Entity;
import java.util.function.BiConsumer;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

/** Tests for the {@link InteractionComponent}. */
Expand Down
50 changes: 24 additions & 26 deletions dungeon/test/contrib/components/InventoryComponentTest.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package contrib.components;

import static org.junit.Assert.*;
import static org.junit.jupiter.api.Assertions.*;

import contrib.item.Item;
import core.Entity;
import core.Game;
import core.utils.components.draw.Animation;
import core.utils.components.path.SimpleIPath;
import java.util.Arrays;
import org.junit.After;
import org.junit.Test;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;

/** Tests for the {@link InventoryComponent}. */
Expand All @@ -20,7 +20,7 @@ public class InventoryComponentTest {
new SimpleIPath("animation/missing_texture.png");

/** WTF? . */
@After
@AfterEach
public void cleanup() {
Game.removeAllEntities();
}
Expand Down Expand Up @@ -125,7 +125,7 @@ public void getAllItemsEmptyInventory() {
Entity e = new Entity();
InventoryComponent ic = new InventoryComponent(0);
e.add(ic);
assertEquals("should have no Items", 0, ic.count());
assertEquals(0, ic.count());
}

/** An inventory with one Item should return a List with this Item. */
Expand All @@ -138,8 +138,8 @@ public void getAllItemsInventoryWithOnlyOneItem() {
new Item("Test item", "Test description", Animation.fromSingleImage(MISSING_TEXTURE));
ic.add(itemData);
Item[] list = ic.items();
assertEquals("should have one Item", 1, list.length);
assertTrue("Item should be in returned List", Arrays.asList(list).contains(itemData));
assertEquals(1, list.length);
assertTrue(Arrays.asList(list).contains(itemData));
}

/** An inventory with one Item should return a List with this Item. */
Expand All @@ -155,9 +155,9 @@ public void getAllItemsInventoryWithTwoItems() {
new Item("Test item", "Test description", Animation.fromSingleImage(MISSING_TEXTURE));
ic.add(itemData2);
Item[] list = ic.items();
assertEquals("should have two Items", 2, list.length);
assertTrue("Item 1 should be in returned List", Arrays.asList(list).contains(itemData1));
assertTrue("Item 2 should be in returned List", Arrays.asList(list).contains(itemData2));
assertEquals(2, list.length);
assertTrue(Arrays.asList(list).contains(itemData1));
assertTrue(Arrays.asList(list).contains(itemData2));
}

/** An inventory should only be able to return Items it contains. */
Expand All @@ -169,8 +169,8 @@ public void getAllItemsInventoryNoAddedItemButCreated() {
Item itemData =
new Item("Test item", "Test description", Animation.fromSingleImage(MISSING_TEXTURE));
Item[] list = ic.items();
assertEquals("should have no Items", 0, ic.count());
assertFalse("Item should not be in returned List", Arrays.asList(list).contains(itemData));
assertEquals(0, ic.count());
assertFalse(Arrays.asList(list).contains(itemData));
}

/** WTF? . */
Expand All @@ -180,12 +180,10 @@ public void tranfserItem() {
InventoryComponent other = new InventoryComponent(1);
Item item = Mockito.mock(Item.class);
ic.add(item);
assertTrue("Item should be in the inventory.", Arrays.asList(ic.items()).contains(item));
assertTrue("Transfer should be successfully.", ic.transfer(item, other));
assertTrue(
"Item should now be in the other inventory.", Arrays.asList(other.items()).contains(item));
assertFalse(
"Item should be removed from this inventroy.", Arrays.asList(ic.items()).contains(item));
assertTrue(Arrays.asList(ic.items()).contains(item));
assertTrue(ic.transfer(item, other));
assertTrue(Arrays.asList(other.items()).contains(item));
assertFalse(Arrays.asList(ic.items()).contains(item));
}

/** WTF? . */
Expand All @@ -195,10 +193,10 @@ public void tranfserItemNoSpace() {
InventoryComponent other = new InventoryComponent(0);
Item item = Mockito.mock(Item.class);
ic.add(item);
assertTrue("Item should be in the inventory.", Arrays.asList(ic.items()).contains(item));
assertFalse("Other inventory is full, no transfer possible", ic.transfer(item, other));
assertFalse("Item should not be transfered", Arrays.asList(other.items()).contains(item));
assertTrue("Item should still be in tis inventroy.", Arrays.asList(ic.items()).contains(item));
assertTrue(Arrays.asList(ic.items()).contains(item));
assertFalse(ic.transfer(item, other));
assertFalse(Arrays.asList(other.items()).contains(item));
assertTrue(Arrays.asList(ic.items()).contains(item));
}

/** WTF? . */
Expand All @@ -207,7 +205,7 @@ public void tranfserItemNoItem() {
InventoryComponent ic = new InventoryComponent(1);
InventoryComponent other = new InventoryComponent(1);
Item item = Mockito.mock(Item.class);
assertFalse("No item, no transfer", ic.transfer(item, other));
assertFalse(ic.transfer(item, other));
}

/** WTF? . */
Expand All @@ -216,8 +214,8 @@ public void transferItemToItself() {
InventoryComponent ic = new InventoryComponent(1);
Item item = Mockito.mock(Item.class);
ic.add(item);
assertTrue("Item should be in the inventory.", Arrays.asList(ic.items()).contains(item));
assertFalse("Can not transfer item to itself.", ic.transfer(item, ic));
assertTrue("Item should still be in tis inventroy.", Arrays.asList(ic.items()).contains(item));
assertTrue(Arrays.asList(ic.items()).contains(item));
assertFalse(ic.transfer(item, ic));
assertTrue(Arrays.asList(ic.items()).contains(item));
}
}
22 changes: 10 additions & 12 deletions dungeon/test/contrib/crafting/CraftingTest.java
Original file line number Diff line number Diff line change
@@ -1,24 +1,22 @@
package contrib.crafting;

import static org.junit.Assert.*;
import static org.junit.jupiter.api.Assertions.*;

import contrib.item.HealthPotionType;
import contrib.item.Item;
import contrib.item.concreteItem.ItemPotionHealth;
import contrib.item.concreteItem.ItemPotionWater;
import contrib.item.concreteItem.ItemResourceMushroomRed;
import java.util.Optional;
import org.junit.Test;
import org.junit.jupiter.api.Test;

/** Tests for the {@link Crafting} class. */
public class CraftingTest {

/** WTF? . */
@Test
public void testFindRecipeWithNoInputs() {
assertTrue(
"No Recipe should be found with no ingredients.",
Crafting.recipeByIngredients(new Item[0]).isEmpty());
assertTrue(Crafting.recipeByIngredients(new Item[0]).isEmpty());
}

/** WTF? . */
Expand All @@ -40,8 +38,8 @@ public void testRecipeFoundUnordered() {

Optional<Recipe> foundRecipe = Crafting.recipeByIngredients(ingredients);

assertFalse("There should be a recipe.", foundRecipe.isEmpty());
assertEquals("The found recipe is the correct recipe", recipe, foundRecipe.get());
assertFalse(foundRecipe.isEmpty());
assertEquals(recipe, foundRecipe.get());

// Cleanup
Crafting.clearRecipes();
Expand All @@ -66,8 +64,8 @@ public void testRecipeOrdered_CorrectOrder() {

Optional<Recipe> foundRecipe = Crafting.recipeByIngredients(ingredients);

assertFalse("There should be a recipe.", foundRecipe.isEmpty());
assertEquals("The found recipe is the correct recipe", recipe, foundRecipe.get());
assertFalse(foundRecipe.isEmpty());
assertEquals(recipe, foundRecipe.get());

// Cleanup
Crafting.clearRecipes();
Expand All @@ -92,7 +90,7 @@ public void testRecipeOrdered_IncorrectOrder() {

Optional<Recipe> foundRecipe = Crafting.recipeByIngredients(ingredients);

assertTrue("There should be no recipe.", foundRecipe.isEmpty());
assertTrue(foundRecipe.isEmpty());

// Cleanup
Crafting.clearRecipes();
Expand All @@ -117,8 +115,8 @@ public void testPrioritizeOrderedRecipes() {
};
Optional<Recipe> foundRecipe = Crafting.recipeByIngredients(ingredients);

assertFalse("There should be a recipe.", foundRecipe.isEmpty());
assertEquals("The found recipe is the correct recipe", recipeOrdered, foundRecipe.get());
assertFalse(foundRecipe.isEmpty());
assertEquals(recipeOrdered, foundRecipe.get());

// Cleanup
Crafting.clearRecipes();
Expand Down
Loading

0 comments on commit 84454da

Please sign in to comment.