Merge branch 'patch-carddb-performance' into 'master'

HOT FIX: CardDb Tests with ImageKeys Interactions

See merge request core-developers/forge!5344

forge-core/src/main/java/forge/card/CardDb.java

@@ -556,9 +556,11 @@ public final class CardDb implements ICardDatabase, IDeckGenPool {
         // Before returning make sure that actual candidate has Image.
         // If not, try to replace current candidate with one having image,
         // so to align this implementation with old one.
-        while (!candidate.hasImage() && candidatesIterator.hasNext()) {
+        // If none will have image, the original candidate will be retained!
+        PaperCard firstCandidate = candidate;
+        while (!candidate.hasImage() && candidatesIterator.hasNext())
             candidate = candidatesIterator.next();
-        }
+        candidate = candidate.hasImage() ? candidate : firstCandidate;
         return isFoil ? candidate.getFoiled() : candidate;
     }
 
@@ -724,10 +726,12 @@ public final class CardDb implements ICardDatabase, IDeckGenPool {
         final Iterator<CardEdition> editionIterator = acceptedEditions.iterator();
         CardEdition ed = editionIterator.next();
         PaperCard candidate = candidatesCard.get(ed.getCode());
+        PaperCard firstCandidate = candidate;
         while (!candidate.hasImage() && editionIterator.hasNext()) {
             ed = editionIterator.next();
             candidate = candidatesCard.get(ed.getCode());
         }
+        candidate = candidate.hasImage() ? candidate : firstCandidate;
         //If any, we're sure that at least one candidate is always returned despite it having any image
         return cr.isFoil ? candidate.getFoiled() : candidate;
     }

forge-gui-desktop/src/test/java/forge/card/CardDbTestCase.java

@@ -7,14 +7,10 @@ import forge.item.PaperCard;
 import forge.model.FModel;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
-
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.time.Instant;
-import java.util.ArrayList;
+import java.util.*;
-import java.util.Arrays;
-import java.util.Date;
-import java.util.List;
 
 import static org.testng.Assert.*;
 
@@ -2138,5 +2134,4 @@ public class CardDbTestCase extends ForgeCardMockTestCase {
         assertTrue(islandCard.isFoil());
     }
 
-}
+}
-

forge-gui-desktop/src/test/java/forge/card/CardDbTestWithNoImage.java

@@ -1,5 +1,6 @@
 package forge.card;
 
+import forge.ImageCache;
 import forge.ImageKeys;
 import forge.item.PaperCard;
 import org.mockito.Mockito;
@@ -7,6 +8,8 @@ import org.powermock.api.mockito.PowerMockito;
 import org.testng.annotations.BeforeMethod;
 import org.testng.annotations.Test;
 
+import javax.imageio.ImageIO;
+
 import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNotNull;
 
@@ -26,11 +29,11 @@ public class CardDbTestWithNoImage extends CardDbTestCase {
     }
 
     @Override
-    @BeforeMethod
+    protected void initCardImageMocks() {
-    protected void initMocks() throws Exception {
+        PowerMockito.mockStatic(ImageIO.class);
+        PowerMockito.mockStatic(ImageCache.class);
         PowerMockito.mockStatic(ImageKeys.class);
-        PowerMockito.when(ImageKeys.hasImage(Mockito.any(PaperCard.class))).thenReturn(false);
+        PowerMockito.when(ImageKeys.hasImage(Mockito.any(PaperCard.class), Mockito.anyBoolean())).thenReturn(false);
-        super.initMocks();
     }
 
     @Test

forge-gui-desktop/src/test/java/forge/card/ForgeCardMockTestCase.java

@@ -114,7 +114,7 @@ public class ForgeCardMockTestCase extends PowerMockTestCase {
         fLangDir.set(ForgeConstants.class, langDir);
     }
 
-    private void setMock(Localizer mock) {
+    protected void setMock(Localizer mock) {
         try {
             Field instance = Localizer.class.getDeclaredField("instance");
             instance.setAccessible(true);
@@ -128,16 +128,14 @@ public class ForgeCardMockTestCase extends PowerMockTestCase {
     protected void initMocks() throws Exception {
         //Loading a card also automatically loads the image, which we do not want (even if it wouldn't cause exceptions).
         //The static initializer block in ImageCache can't fully be mocked (https://code.google.com/p/powermock/issues/detail?id=256), so we also need to mess with ImageIO...
-        //TODO: make sure that loading images only happens in a GUI environment, so we no longer need to mock this
+        initCardImageMocks();
-        PowerMockito.mockStatic(ImageIO.class);
-        PowerMockito.mockStatic(ImageCache.class);
-        PowerMockito.mockStatic(ImageKeys.class);
         initForgeConstants();
-
-        // Always Has Image (there is a separated test case to cover the opposite case)
-        PowerMockito.when(ImageKeys.hasImage(Mockito.any(PaperCard.class))).thenReturn(true);
-
         //Mocking some more static stuff
+        initForgePreferences();
+        initializeStaticData();
+    }
+
+    protected void initForgePreferences() throws IllegalAccessException {
         PowerMockito.mockStatic(Singletons.class);
         PowerMockito.mockStatic(FModel.class);
         ForgePreferences forgePreferences = new ForgePreferences();
@@ -161,7 +159,14 @@ public class ForgeCardMockTestCase extends PowerMockTestCase {
         PowerMockito.field(Localizer.class, "resourceBundle").set(localizerMock, dummyResourceBundle);
         PowerMockito.when(localizerMock.getMessage(Mockito.anyString())).thenReturn("any string");
         PowerMockito.when(FModel.getPreferences()).thenReturn(forgePreferences);
-        initializeStaticData();
+    }
+
+    protected void initCardImageMocks() {
+        //make sure that loading images only happens in a GUI environment, so we no longer need to mock this
+        PowerMockito.mockStatic(ImageIO.class);
+        PowerMockito.mockStatic(ImageCache.class);
+        PowerMockito.mockStatic(ImageKeys.class);
+        PowerMockito.when(ImageKeys.hasImage(Mockito.any(PaperCard.class), Mockito.anyBoolean())).thenReturn(true);
     }
 
     protected void initializeStaticData() {