Clean up CasualtyDetailsTest and switch to AssertJ. (#12684)

Note: See #12121 for discussion where we decided on moving to AssertJ. This is the first adoption of it. I'm updating this test ahead of expanding test coverage with a fix for 3hp unit casualty logic.
triplea-game · Jul 4, 2024 · 14b6ca4 · 14b6ca4
1 parent b0d468e
commit 14b6ca4
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 62 deletions.
diff --git a/build.gradle b/build.gradle
@@ -46,6 +46,7 @@ subprojects {
 
     ext {
         apacheHttpComponentsVersion = "4.5.14"
+        assertjCoreVersion = '3.11.1'
         awaitilityVersion = "4.2.1"
         bcryptVersion = "0.10.2"
         caffeineVersion = "3.1.8"
@@ -100,6 +101,7 @@ subprojects {
     dependencies {
         implementation "ch.qos.logback:logback-classic:$logbackClassicVersion"
         implementation "com.google.guava:guava:$guavaVersion"
+        testImplementation "org.assertj:assertj-core:$assertjCoreVersion"
         testImplementation "com.github.npathai:hamcrest-optional:$hamcrestOptionalVersion"
         testImplementation "nl.jqno.equalsverifier:equalsverifier:$equalsVerifierVersion"
         testImplementation "org.hamcrest:java-hamcrest:$hamcrestVersion"

diff --git a/...app/game-core/src/test/java/games/strategy/triplea/delegate/data/CasualtyDetailsTest.java b/...app/game-core/src/test/java/games/strategy/triplea/delegate/data/CasualtyDetailsTest.java
@@ -2,9 +2,7 @@
 
 import static games.strategy.triplea.Constants.UNIT_ATTACHMENT_NAME;
 import static games.strategy.triplea.delegate.battle.steps.MockGameData.givenGameData;
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.containsInAnyOrder;
-import static org.hamcrest.Matchers.is;
+import static org.assertj.core.api.Assertions.assertThat;
 
 import games.strategy.engine.data.GameData;
 import games.strategy.engine.data.GamePlayer;
@@ -22,7 +20,6 @@
 
 @ExtendWith(MockitoExtension.class)
 class CasualtyDetailsTest {
-
   private final GameData gameData = givenGameData().build();
   private final GamePlayer player1 = new GamePlayer("player1", gameData);
   private final GamePlayer player2 = new GamePlayer("player2", gameData);
@@ -38,38 +35,32 @@ private UnitType givenUnitType(final String name) {
   void ignoreNonAirUnitsAlreadyKilled() {
     final UnitType infantry = givenUnitType("infantry");
 
-    final List<Unit> units = new ArrayList<>();
-    units.addAll(infantry.createTemp(2, player1));
+    final List<Unit> units = infantry.createTemp(2, player1);
 
     final List<Unit> killed = new ArrayList<>();
     killed.add(units.get(0));
 
     final CasualtyDetails casualtyDetails = new CasualtyDetails(killed, List.of(), true);
     casualtyDetails.ensureUnitsAreKilledFirst(
         units, Matches.unitIsAir(), Comparator.comparing(Unit::getMovementLeft));
-    assertThat(
-        "The infantry should still be killed",
-        casualtyDetails.getKilled(),
-        is(containsInAnyOrder(units.get(0))));
+    // The infantry should still be killed.
+    assertThat(casualtyDetails.getKilled()).containsExactly(units.get(0));
   }
 
   @Test
   void ignoreNonAirUnitsAlreadyDamaged() {
     final UnitType infantry = givenUnitType("infantry");
 
-    final List<Unit> units = new ArrayList<>();
-    units.addAll(infantry.createTemp(2, player1));
+    final List<Unit> units = infantry.createTemp(2, player1);
 
     final List<Unit> damaged = new ArrayList<>();
     damaged.add(units.get(0));
 
     final CasualtyDetails casualtyDetails = new CasualtyDetails(List.of(), damaged, true);
     casualtyDetails.ensureUnitsAreDamagedFirst(
         units, Matches.unitIsAir(), Comparator.comparing(Unit::getMovementLeft).reversed());
-    assertThat(
-        "The infantry should still be damaged",
-        casualtyDetails.getDamaged(),
-        is(containsInAnyOrder(units.get(0))));
+    // The infantry should still be damaged.
+    assertThat(casualtyDetails.getDamaged()).containsExactly(units.get(0));
   }
 
   @Test
@@ -78,8 +69,7 @@ void killLowestMovementAirUnitsWhenOnlyOneTypeIsAvailable() {
     fighter.getUnitAttachment().setMovement(4);
     fighter.getUnitAttachment().setIsAir(true);
 
-    final List<Unit> units = new ArrayList<>();
-    units.addAll(fighter.createTemp(2, player1));
+    final List<Unit> units = fighter.createTemp(2, player1);
 
     units.get(0).setAlreadyMoved(BigDecimal.ONE);
     units.get(1).setAlreadyMoved(BigDecimal.valueOf(2));
@@ -90,10 +80,8 @@ void killLowestMovementAirUnitsWhenOnlyOneTypeIsAvailable() {
     final CasualtyDetails casualtyDetails = new CasualtyDetails(killed, List.of(), true);
     casualtyDetails.ensureUnitsAreKilledFirst(
         units, Matches.unitIsAir(), Comparator.comparing(Unit::getMovementLeft));
-    assertThat(
-        "The second air unit has no movement left so it should be killed",
-        casualtyDetails.getKilled(),
-        is(containsInAnyOrder(units.get(1))));
+    // The second air unit has no movement left so it should be killed.
+    assertThat(casualtyDetails.getKilled()).containsExactly(units.get(1));
   }
 
   @Test
@@ -103,8 +91,7 @@ void damageHighestMovementAirUnitsWhenOnlyOneTypeIsAvailable() {
     fighter.getUnitAttachment().setMovement(4);
     fighter.getUnitAttachment().setIsAir(true);
 
-    final List<Unit> units = new ArrayList<>();
-    units.addAll(fighter.createTemp(2, player1));
+    final List<Unit> units = fighter.createTemp(2, player1);
 
     units.get(0).setAlreadyMoved(BigDecimal.ONE);
     units.get(1).setAlreadyMoved(BigDecimal.valueOf(2));
@@ -116,10 +103,8 @@ void damageHighestMovementAirUnitsWhenOnlyOneTypeIsAvailable() {
     casualtyDetails.ensureUnitsAreDamagedFirst(
         units, Matches.unitIsAir(), Comparator.comparing(Unit::getMovementLeft).reversed());
 
-    assertThat(
-        "The first air unit has one movement left so it should be damaged",
-        casualtyDetails.getDamaged(),
-        is(containsInAnyOrder(units.get(0))));
+    // The first air unit has one movement left so it should be damaged.
+    assertThat(casualtyDetails.getDamaged()).containsExactly(units.get(0));
   }
 
   @Test
@@ -149,10 +134,8 @@ void killLowestMovementAirUnitsInTwoTypes() {
     casualtyDetails.ensureUnitsAreKilledFirst(
         units, Matches.unitIsAir(), Comparator.comparing(Unit::getMovementLeft));
 
-    assertThat(
-        "The second and fourth air unit have no movement left so it should be killed",
-        casualtyDetails.getKilled(),
-        is(containsInAnyOrder(units.get(1), units.get(3))));
+    // The second and fourth air unit have no movement left so it should be killed.
+    assertThat(casualtyDetails.getKilled()).containsExactlyInAnyOrder(units.get(1), units.get(3));
   }
 
   @Test
@@ -184,10 +167,8 @@ void damageHighestMovementAirUnitsInTwoTypes() {
     casualtyDetails.ensureUnitsAreDamagedFirst(
         units, Matches.unitIsAir(), Comparator.comparing(Unit::getMovementLeft).reversed());
 
-    assertThat(
-        "The first and third air unit have one movement left so they should be damaged",
-        casualtyDetails.getDamaged(),
-        is(containsInAnyOrder(units.get(0), units.get(2))));
+    // The first and third air unit have one movement left so they should be damaged.
+    assertThat(casualtyDetails.getDamaged()).containsExactlyInAnyOrder(units.get(0), units.get(2));
   }
 
   @Test
@@ -213,10 +194,8 @@ void damageHighestMovementAirUnitsWithTwoOwners() {
     casualtyDetails.ensureUnitsAreDamagedFirst(
         units, Matches.unitIsAir(), Comparator.comparing(Unit::getMovementLeft).reversed());
 
-    assertThat(
-        "Damage is not distributed to a unit of another owner",
-        casualtyDetails.getDamaged(),
-        is(containsInAnyOrder(units.get(1), units.get(0))));
+    // Damage is not distributed to a unit of another owner.
+    assertThat(casualtyDetails.getDamaged()).containsExactlyInAnyOrder(units.get(1), units.get(0));
   }
 
   @Test
@@ -226,8 +205,7 @@ void keepDamageAtUnitsAlreadyDamaged() {
     fighter.getUnitAttachment().setMovement(4);
     fighter.getUnitAttachment().setIsAir(true);
 
-    final List<Unit> units = new ArrayList<>();
-    units.addAll(fighter.createTemp(2, player1));
+    final List<Unit> units = fighter.createTemp(2, player1);
 
     units.get(0).setAlreadyMoved(BigDecimal.ONE);
     units.get(1).setAlreadyMoved(BigDecimal.valueOf(2));
@@ -240,21 +218,18 @@ void keepDamageAtUnitsAlreadyDamaged() {
     casualtyDetails.ensureUnitsAreDamagedFirst(
         units, Matches.unitIsAir(), Comparator.comparing(Unit::getMovementLeft).reversed());
 
-    assertThat(
-        "The first and third air unit have one movement left so they should be damaged",
-        casualtyDetails.getDamaged().size() == 1
-            && casualtyDetails.getDamaged().contains(units.get(1))
-            && units.get(0).getHits() == 1
-            && units.get(1).getHits() == 0);
+    // The first and third air unit have one movement left so they should be damaged.
+    assertThat(casualtyDetails.getDamaged()).containsExactly(units.get(1));
+    assertThat(units.get(0).getHits()).isEqualTo(1);
+    assertThat(units.get(1).getHits()).isEqualTo(0);
   }
 
   @Test
   void killPositiveMarineBonusLastIfAmphibious() {
     final UnitType infantry = givenUnitType("infantry");
     infantry.getUnitAttachment().setIsMarine(1);
 
-    final List<Unit> units = new ArrayList<>();
-    units.addAll(infantry.createTemp(2, player1));
+    final List<Unit> units = infantry.createTemp(2, player1);
 
     units.get(0).setWasAmphibious(true);
     units.get(1).setWasAmphibious(false);
@@ -264,20 +239,17 @@ void killPositiveMarineBonusLastIfAmphibious() {
 
     final CasualtyDetails casualtyDetails = new CasualtyDetails(killed, List.of(), true);
     casualtyDetails.ensureUnitsWithPositiveMarineBonusAreKilledLast(units);
-    assertThat(
-        "The second unit was not amphibious, so it doesn't have the positive marine bonus "
-            + "and should be taken first.",
-        casualtyDetails.getKilled(),
-        is(containsInAnyOrder(units.get(1))));
+    // The second unit was not amphibious, so it doesn't have the positive marine bonus and should
+    // be taken first.
+    assertThat(casualtyDetails.getKilled()).containsExactly(units.get(1));
   }
 
   @Test
   void killNegativeMarineBonusFirstIfAmphibious() {
     final UnitType infantry = givenUnitType("infantry");
     infantry.getUnitAttachment().setIsMarine(-1);
 
-    final List<Unit> units = new ArrayList<>();
-    units.addAll(infantry.createTemp(2, player1));
+    final List<Unit> units = infantry.createTemp(2, player1);
 
     units.get(0).setWasAmphibious(false);
     units.get(1).setWasAmphibious(true);
@@ -287,10 +259,8 @@ void killNegativeMarineBonusFirstIfAmphibious() {
 
     final CasualtyDetails casualtyDetails = new CasualtyDetails(killed, List.of(), true);
     casualtyDetails.ensureUnitsWithPositiveMarineBonusAreKilledLast(units);
-    assertThat(
-        "The second unit was amphibious, so it has the negative marine bonus "
-            + "and should be taken first.",
-        casualtyDetails.getKilled(),
-        is(containsInAnyOrder(units.get(1))));
+    // The second unit was amphibious, so it has the negative marine bonus and should be taken
+    // first.
+    assertThat(casualtyDetails.getKilled()).containsExactly(units.get(1));
   }
 }