From e9c68d2a5cc1a9ed2afe97ea2f1953fd0663ded7 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Sat, 21 Aug 2021 15:02:42 +0400 Subject: [PATCH] Test framework improves (related to #7896): * added real time command to check card's counters (example: suspended cards with Time counters, see checkCardCounters); * added target type support: TargetPermanentOrSuspendedCard; * improves error logs for miss modes and unsupported target types; --- .../java/org/mage/test/player/TestPlayer.java | 94 +++++++++++++++---- .../base/impl/CardTestPlayerAPIImpl.java | 5 + .../TargetPermanentOrSuspendedCard.java | 31 +----- 3 files changed, 80 insertions(+), 50 deletions(-) diff --git a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java index d702521448..4411373ea3 100644 --- a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java +++ b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java @@ -342,7 +342,7 @@ public class TestPlayer implements Player { for (Player player : game.getPlayers().values()) { if (player.getName().equals(target)) { if (ability.getTargets().isEmpty()) { - throw new UnsupportedOperationException("Ability has no targets, but there is a player target set - " + ability.toString()); + throw new UnsupportedOperationException("Ability has no targets, but there is a player target set - " + ability); } if (ability.getTargets().get(0) instanceof TargetAmount) { return true; // targetAmount have to be set by setTargetAmount in the test script @@ -461,10 +461,10 @@ public class TestPlayer implements Player { selectedMode = ability.getModes().getMode(); } if (selectedMode == null) { - throw new UnsupportedOperationException("Mode not available for " + ability.toString()); + throw new UnsupportedOperationException("Mode not available for " + ability); } if (selectedMode.getTargets().isEmpty()) { - throw new AssertionError("Ability has no targets. " + ability.toString()); + throw new AssertionError("Ability has no targets. " + ability); } if (index >= selectedMode.getTargets().size()) { break; // this can happen if targets should be set but can't be used because of hexproof e.g. @@ -821,6 +821,13 @@ public class TestPlayer implements Player { wasProccessed = true; } + // check card counters: card name, counter type, count + if (params[0].equals(CHECK_COMMAND_CARD_COUNTERS) && params.length == 4) { + assertCardCounters(action, game, computerPlayer, params[1], CounterType.findByName(params[2]), Integer.parseInt(params[3])); + actions.remove(action); + wasProccessed = true; + } + // check exile count: card name, count if (params[0].equals(CHECK_COMMAND_EXILE_COUNT) && params.length == 3) { assertExileCount(action, game, params[1], Integer.parseInt(params[2])); @@ -1365,6 +1372,25 @@ public class TestPlayer implements Player { Assert.assertEquals(action.getActionName() + " - permanent " + permanentName + " must have " + count + " " + counterType.toString(), count, foundCount); } + private void assertCardCounters(PlayerAction action, Game game, Player player, String cardName, CounterType counterType, int count) { + int foundCount = 0; + + Set allCards = new HashSet<>(); + + // collect possible cards from visible zones except library + allCards.addAll(player.getHand().getCards(game)); + allCards.addAll(player.getGraveyard().getCards(game)); + allCards.addAll(game.getExile().getAllCards(game)); + + for (Card card : allCards) { + if (hasObjectTargetNameOrAlias(card, cardName) && card.getOwnerId().equals(player.getId())) { + foundCount = card.getCounters(game).getCount(counterType); + } + } + + Assert.assertEquals(action.getActionName() + " - card " + cardName + " must have " + count + " " + counterType.toString(), count, foundCount); + } + private void assertExileCount(PlayerAction action, Game game, String permanentName, int count) { int foundCount = 0; for (Card card : game.getExile().getAllCards(game)) { @@ -1465,9 +1491,9 @@ public class TestPlayer implements Player { } if (mustHave) { - Assert.assertEquals(action.getActionName() + " - must contain colors [" + searchColors.toString() + "] but found only [" + cardColor.toString() + "]", 0, colorsDontHave.size()); + Assert.assertEquals(action.getActionName() + " - must contain colors [" + searchColors + "] but found only [" + cardColor.toString() + "]", 0, colorsDontHave.size()); } else { - Assert.assertEquals(action.getActionName() + " - must not contain colors [" + searchColors.toString() + "] but found [" + cardColor.toString() + "]", 0, colorsHave.size()); + Assert.assertEquals(action.getActionName() + " - must not contain colors [" + searchColors + "] but found [" + cardColor.toString() + "]", 0, colorsHave.size()); } } @@ -1558,7 +1584,7 @@ public class TestPlayer implements Player { Integer normal = player.getManaPool().getMana().get(manaType); Integer conditional = player.getManaPool().getConditionalMana().stream().mapToInt(a -> a.get(manaType)).sum(); // calcs FULL conditional mana, not real conditions Integer current = normal + conditional; - Assert.assertEquals(action.getActionName() + " - mana pool must contain [" + amount.toString() + " " + manaType.toString() + "], but found [" + current.toString() + "]", amount, current); + Assert.assertEquals(action.getActionName() + " - mana pool must contain [" + amount.toString() + " " + manaType + "], but found [" + current + "]", amount, current); } private void assertManaPool(PlayerAction action, Game game, Player player, String colors, Integer amount) { @@ -1952,7 +1978,18 @@ public class TestPlayer implements Player { return null; } - this.chooseStrictModeFailed("mode", game, getInfo(source, game)); + StringBuilder modesInfo = new StringBuilder(); + modesInfo.append("\nAvailable modes:"); + int i = 1; + for (Mode mode : modes.getAvailableModes(source, game)) { + if (modesInfo.length() > 0) { + modesInfo.append("\n"); + } + modesInfo.append(String.format("%d: %s", i, mode.getEffects().getText(mode))); + i++; + } + + this.chooseStrictModeFailed("mode", game, getInfo(source, game) + modesInfo); return computerPlayer.chooseMode(modes, source, game); } @@ -2298,7 +2335,8 @@ public class TestPlayer implements Player { || (target.getOriginalTarget() instanceof TargetPermanentOrPlayer) || (target.getOriginalTarget() instanceof TargetAnyTarget) || (target.getOriginalTarget() instanceof TargetCreatureOrPlayer) - || (target.getOriginalTarget() instanceof TargetDefender)) { + || (target.getOriginalTarget() instanceof TargetDefender) + || (target.getOriginalTarget() instanceof TargetPermanentOrSuspendedCard)) { for (String targetDefinition : targets) { if (targetDefinition.startsWith("targetPlayer=")) { continue; @@ -2330,6 +2368,9 @@ public class TestPlayer implements Player { if (filter instanceof FilterPlaneswalkerOrPlayer) { filter = ((FilterPlaneswalkerOrPlayer) filter).getFilterPermanent(); } + if (filter instanceof FilterPermanentOrSuspendedCard) { + filter = ((FilterPermanentOrSuspendedCard) filter).getPermanentFilter(); + } for (Permanent permanent : game.getBattlefield().getActivePermanents((FilterPermanent) filter, abilityControllerId, sourceId, game)) { if (hasObjectTargetNameOrAlias(permanent, targetName) || (permanent.getName() + '-' + permanent.getExpansionSetCode()).equals(targetName)) { // TODO: remove exp code search? if (target.canTarget(abilityControllerId, permanent.getId(), source, game) && !target.getTargets().contains(permanent.getId())) { @@ -2375,14 +2416,26 @@ public class TestPlayer implements Player { } // card in exile - if (target.getOriginalTarget() instanceof TargetCardInExile) { - TargetCardInExile targetFull = (TargetCardInExile) target.getOriginalTarget(); + if (target.getOriginalTarget() instanceof TargetCardInExile + || target.getOriginalTarget() instanceof TargetPermanentOrSuspendedCard) { + + FilterCard filterCard = null; + if (target.getOriginalTarget().getFilter() instanceof FilterCard) { + filterCard = (FilterCard) target.getOriginalTarget().getFilter(); + } else if (target.getOriginalTarget().getFilter() instanceof FilterPermanentOrSuspendedCard) { + filterCard = ((FilterPermanentOrSuspendedCard) target.getOriginalTarget().getFilter()).getCardFilter(); + } + if (filterCard == null) { + Assert.fail("Unsupported exile target filter in TestPlayer: " + + target.getOriginalTarget().getClass().getCanonicalName()); + } + for (String targetDefinition : targets) { checkTargetDefinitionMarksSupport(target, targetDefinition, "^"); String[] targetList = targetDefinition.split("\\^"); boolean targetFound = false; for (String targetName : targetList) { - for (Card card : game.getExile().getCards(targetFull.getFilter(), game)) { + for (Card card : game.getExile().getCards(filterCard, game)) { if (hasObjectTargetNameOrAlias(card, targetName) || (card.getName() + '-' + card.getExpansionSetCode()).equals(targetName)) { // TODO: remove set code search? if (target.canTarget(abilityControllerId, card.getId(), source, game) && !target.getTargets().contains(card.getId())) { target.addTarget(card.getId(), source, game); @@ -2508,16 +2561,17 @@ public class TestPlayer implements Player { String message; if (source != null) { - message = this.getName() + " - Targets list was setup by addTarget with " + targets + ", but not used in [" - + "card " + source.getSourceObject(game) - + " -> ability " + source.getClass().getSimpleName() + " (" + source.getRule().substring(0, Math.min(20, source.getRule().length()) - 1) + "..." + ")" - + " -> target " + target.getClass().getSimpleName() + " (" + target.getMessage() + ")" - + "]"; + message = this.getName() + " - Targets list was setup by addTarget with " + targets + ", but not used" + + "\nCard: " + source.getSourceObject(game) + + "\nAbility: " + source.getClass().getSimpleName() + " (" + source.getRule() + ")" + + "\nTarget: " + target.getClass().getSimpleName() + " (" + target.getMessage() + ")" + + "\nYou must implement target class support in TestPlayer or setup good targets"; } else { - message = this.getName() + " - Targets list was setup by addTarget with " + targets + ", but not used in [" - + "card XXX" - + " -> target " + target.getClass().getSimpleName() + " (" + target.getMessage() + ")" - + "]"; + message = this.getName() + " - Targets list was setup by addTarget with " + targets + ", but not used" + + "\nCard: unknown source" + + "\nAbility: unknown source" + + "\nTarget: " + target.getClass().getSimpleName() + " (" + target.getMessage() + ")" + + "\nYou must implement target class support in TestPlayer or setup good targets"; } Assert.fail(message); } diff --git a/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java b/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java index 1ed8cad43c..f9e9162d37 100644 --- a/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java +++ b/Mage.Tests/src/test/java/org/mage/test/serverside/base/impl/CardTestPlayerAPIImpl.java @@ -97,6 +97,7 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement public static final String CHECK_COMMAND_PERMANENT_COUNT = "PERMANENT_COUNT"; public static final String CHECK_COMMAND_PERMANENT_TAPPED = "PERMANENT_TAPPED"; public static final String CHECK_COMMAND_PERMANENT_COUNTERS = "PERMANENT_COUNTERS"; + public static final String CHECK_COMMAND_CARD_COUNTERS = "CARD_COUNTERS"; public static final String CHECK_COMMAND_EXILE_COUNT = "EXILE_COUNT"; public static final String CHECK_COMMAND_GRAVEYARD_COUNT = "GRAVEYARD_COUNT"; public static final String CHECK_COMMAND_LIBRARY_COUNT = "LIBRARY_COUNT"; @@ -457,6 +458,10 @@ public abstract class CardTestPlayerAPIImpl extends MageTestPlayerBase implement check(checkName, turnNum, step, player, CHECK_COMMAND_PERMANENT_COUNTERS, permanentName, counterType.toString(), count.toString()); } + public void checkCardCounters(String checkName, int turnNum, PhaseStep step, TestPlayer player, String cardName, CounterType counterType, Integer count) { + check(checkName, turnNum, step, player, CHECK_COMMAND_CARD_COUNTERS, cardName, counterType.toString(), count.toString()); + } + public void checkExileCount(String checkName, int turnNum, PhaseStep step, TestPlayer player, String permanentName, Integer count) { //Assert.assertNotEquals("", permanentName); check(checkName, turnNum, step, player, CHECK_COMMAND_EXILE_COUNT, permanentName, count.toString()); diff --git a/Mage/src/main/java/mage/target/common/TargetPermanentOrSuspendedCard.java b/Mage/src/main/java/mage/target/common/TargetPermanentOrSuspendedCard.java index 186d5bc6b3..64d399238a 100644 --- a/Mage/src/main/java/mage/target/common/TargetPermanentOrSuspendedCard.java +++ b/Mage/src/main/java/mage/target/common/TargetPermanentOrSuspendedCard.java @@ -1,32 +1,3 @@ -/* - * - * Copyright 2010 BetaSteward_at_googlemail.com. All rights reserved. - * - * Redistribution and use in source and binary forms, with or without modification, are - * permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, this list of - * conditions and the following disclaimer. - * - * 2. Redistributions in binary form must reproduce the above copyright notice, this list - * of conditions and the following disclaimer in the documentation and/or other materials - * provided with the distribution. - * - * THIS SOFTWARE IS PROVIDED BY BetaSteward_at_googlemail.com ``AS IS'' AND ANY EXPRESS OR IMPLIED - * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND - * FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL BetaSteward_at_googlemail.com OR - * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR - * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR - * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING - * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF - * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * The views and conclusions contained in the software and documentation are those of the - * authors and should not be interpreted as representing official policies, either expressed - * or implied, of BetaSteward_at_googlemail.com. - * - */ package mage.target.common; import mage.MageObject; @@ -154,7 +125,7 @@ public class TargetPermanentOrSuspendedCard extends TargetImpl { @Override public String getTargetedName(Game game) { - StringBuilder sb = new StringBuilder(""); + StringBuilder sb = new StringBuilder(); for (UUID targetId : this.getTargets()) { Permanent permanent = game.getPermanent(targetId); if (permanent != null) {