From 25910269ea15199e5a7180b37997f64ceee8c268 Mon Sep 17 00:00:00 2001 From: asvitkine Date: Thu, 4 Jul 2024 11:28:22 -0400 Subject: [PATCH] Clean up CasualtyDetailsTest and switch to AssertJ. Note: See https://github.com/triplea-game/triplea/discussions/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. --- build.gradle | 2 + .../delegate/data/CasualtyDetailsTest.java | 94 +++++++------------ 2 files changed, 34 insertions(+), 62 deletions(-) diff --git a/build.gradle b/build.gradle index 257fa1ba809..ded3d62e5c1 100644 --- 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/game-app/game-core/src/test/java/games/strategy/triplea/delegate/data/CasualtyDetailsTest.java b/game-app/game-core/src/test/java/games/strategy/triplea/delegate/data/CasualtyDetailsTest.java index 7b6cb115ad4..b2aec17f0c3 100644 --- a/game-app/game-core/src/test/java/games/strategy/triplea/delegate/data/CasualtyDetailsTest.java +++ b/game-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,8 +35,7 @@ private UnitType givenUnitType(final String name) { void ignoreNonAirUnitsAlreadyKilled() { final UnitType infantry = givenUnitType("infantry"); - final List units = new ArrayList<>(); - units.addAll(infantry.createTemp(2, player1)); + final List units = infantry.createTemp(2, player1); final List killed = new ArrayList<>(); killed.add(units.get(0)); @@ -47,18 +43,15 @@ void ignoreNonAirUnitsAlreadyKilled() { 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 units = new ArrayList<>(); - units.addAll(infantry.createTemp(2, player1)); + final List units = infantry.createTemp(2, player1); final List damaged = new ArrayList<>(); damaged.add(units.get(0)); @@ -66,10 +59,8 @@ void ignoreNonAirUnitsAlreadyDamaged() { 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 units = new ArrayList<>(); - units.addAll(fighter.createTemp(2, player1)); + final List 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 units = new ArrayList<>(); - units.addAll(fighter.createTemp(2, player1)); + final List 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 units = new ArrayList<>(); - units.addAll(fighter.createTemp(2, player1)); + final List units = fighter.createTemp(2, player1); units.get(0).setAlreadyMoved(BigDecimal.ONE); units.get(1).setAlreadyMoved(BigDecimal.valueOf(2)); @@ -240,12 +218,10 @@ 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 @@ -253,8 +229,7 @@ void killPositiveMarineBonusLastIfAmphibious() { final UnitType infantry = givenUnitType("infantry"); infantry.getUnitAttachment().setIsMarine(1); - final List units = new ArrayList<>(); - units.addAll(infantry.createTemp(2, player1)); + final List units = infantry.createTemp(2, player1); units.get(0).setWasAmphibious(true); units.get(1).setWasAmphibious(false); @@ -264,11 +239,9 @@ 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 @@ -276,8 +249,7 @@ void killNegativeMarineBonusFirstIfAmphibious() { final UnitType infantry = givenUnitType("infantry"); infantry.getUnitAttachment().setIsMarine(-1); - final List units = new ArrayList<>(); - units.addAll(infantry.createTemp(2, player1)); + final List 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)); } }