tests: add test for item trace (#735)

* tests: add test for item trace

Item Trace has some interesting behaviour: cancelling a previous trace
doesn't take effect until the audience is garbage collected. I left this
as there's no "unregisterListener" function already.

* chore: spotless

* refactor: simplify request logger testing

* fix: unregister listeners immediately

* chore: spotless

* refactor: change ptrace the same way

* tests: cover invalid item

src/net/sourceforge/kolmafia/listener/ItemListenerRegistry.java

@@ -4,17 +4,19 @@ public class ItemListenerRegistry extends ListenerRegistry {
   // The registry of listeners:
   private static final ListenerRegistry INSTANCE = new ListenerRegistry();
 
-  public static final void registerItemListener(final int itemId, final Listener listener) {
+  public static void registerItemListener(final int itemId, final Listener listener) {
     if (itemId < 1) {
       return;
     }
 
-    Integer key = itemId;
-    ItemListenerRegistry.INSTANCE.registerListener(key, listener);
+    ItemListenerRegistry.INSTANCE.registerListener(itemId, listener);
   }
 
-  public static final void fireItemChanged(final int itemId) {
-    Integer key = itemId;
-    ItemListenerRegistry.INSTANCE.fireListener(key);
+  public static void unregisterItemListener(final int itemId, final Listener listener) {
+    ItemListenerRegistry.INSTANCE.unregisterListener(itemId, listener);
+  }
+
+  public static void fireItemChanged(final int itemId) {
+    ItemListenerRegistry.INSTANCE.fireListener(itemId);
   }
 }

src/net/sourceforge/kolmafia/listener/ListenerRegistry.java

@@ -79,6 +79,22 @@ public final void registerListener(final Object key, final Listener listener) {
     }
   }
 
+  public final void unregisterListener(final Object key, final Listener listener) {
+    ArrayList<WeakReference<Listener>> listenerList;
+
+    synchronized (this.listenerMap) {
+      listenerList = this.listenerMap.get(key);
+    }
+
+    if (listenerList == null) {
+      return;
+    }
+
+    synchronized (listenerList) {
+      listenerList.removeIf(ref -> listener.equals(ref.get()));
+    }
+  }
+
   public final void fireListener(final Object key) {
     ArrayList<WeakReference<Listener>> listenerList = null;
 

src/net/sourceforge/kolmafia/listener/PreferenceListenerRegistry.java

@@ -8,15 +8,19 @@ public static void deferPreferenceListeners(boolean deferring) {
     PreferenceListenerRegistry.INSTANCE.deferListeners(deferring);
   }
 
-  public static final void registerPreferenceListener(final String name, final Listener listener) {
+  public static void registerPreferenceListener(final String name, final Listener listener) {
     PreferenceListenerRegistry.INSTANCE.registerListener(name, listener);
   }
 
-  public static final void firePreferenceChanged(final String name) {
+  public static void unregisterPreferenceListener(String name, final Listener listener) {
+    PreferenceListenerRegistry.INSTANCE.unregisterListener(name, listener);
+  }
+
+  public static void firePreferenceChanged(final String name) {
     PreferenceListenerRegistry.INSTANCE.fireListener(name);
   }
 
-  public static final void fireAllPreferencesChanged() {
+  public static void fireAllPreferencesChanged() {
     PreferenceListenerRegistry.INSTANCE.fireAllListeners();
   }
 }

src/net/sourceforge/kolmafia/textui/command/ItemTraceCommand.java

@@ -10,25 +10,28 @@
 import net.sourceforge.kolmafia.persistence.ItemFinder;
 
 public class ItemTraceCommand extends AbstractCommand {
-  private static ArrayList<Listener> audience = null; // keeps listeners from being GC'd
+  private static final ArrayList<ItemListener> audience =
+      new ArrayList<>(); // keeps listeners from being GC'd
 
   public ItemTraceCommand() {
     this.usage = " <item> [, <item>]... - watch changes to inventory count of items";
   }
 
   @Override
   public synchronized void run(String command, final String parameters) {
-    if (audience != null) {
-      audience = null;
+    if (audience.size() != 0) {
+      for (ItemListener listener : audience) {
+        listener.unregister();
+      }
+      audience.clear();
       RequestLogger.printLine("Previously watched items have been cleared.");
     }
 
     if (parameters.equals("")) {
       return;
     }
 
-    AdventureResult[] items = ItemFinder.getMatchingItemList(parameters, KoLConstants.inventory);
-    audience = new ArrayList<Listener>();
+    AdventureResult[] items = ItemFinder.getMatchingItemList(parameters);
     for (AdventureResult item : items) {
       audience.add(new ItemListener(item));
     }
@@ -43,6 +46,10 @@ public ItemListener(AdventureResult item) {
       this.update();
     }
 
+    public void unregister() {
+      ItemListenerRegistry.unregisterItemListener(item.getItemId(), this);
+    }
+
     @Override
     public void update() {
       String msg = "itrace: " + this.item.getName() + " = " + item.getCount(KoLConstants.inventory);

src/net/sourceforge/kolmafia/textui/command/PrefTraceCommand.java

@@ -8,16 +8,20 @@
 import net.sourceforge.kolmafia.preferences.Preferences;
 
 public class PrefTraceCommand extends AbstractCommand {
-  private static ArrayList<Listener> audience = null; // keeps listeners from being GC'd
+  private static final ArrayList<PreferenceListener> audience =
+      new ArrayList<>(); // keeps listeners from being GC'd
 
   public PrefTraceCommand() {
     this.usage = " <name> [, <name>]... - watch changes to indicated preferences";
   }
 
   @Override
   public synchronized void run(String command, final String parameters) {
-    if (audience != null) {
-      audience = null;
+    if (audience.size() != 0) {
+      for (PreferenceListener listener : audience) {
+        listener.unregister();
+      }
+      audience.clear();
       RequestLogger.printLine("Previously watched prefs have been cleared.");
     }
 
@@ -26,9 +30,8 @@ public synchronized void run(String command, final String parameters) {
     }
 
     String[] prefList = parameters.split("\\s*,\\s*");
-    audience = new ArrayList<Listener>();
-    for (int i = 0; i < prefList.length; ++i) {
-      audience.add(new PreferenceListener(prefList[i]));
+    for (String pref : prefList) {
+      audience.add(new PreferenceListener(pref));
     }
   }
 
@@ -41,6 +44,10 @@ public PreferenceListener(String name) {
       this.update();
     }
 
+    public void unregister() {
+      PreferenceListenerRegistry.unregisterPreferenceListener(name, this);
+    }
+
     @Override
     public void update() {
       String msg = "ptrace: " + this.name + " = " + Preferences.getString(this.name);

test/internal/helpers/Player.java

@@ -20,6 +20,7 @@
 import net.sourceforge.kolmafia.persistence.EquipmentDatabase;
 import net.sourceforge.kolmafia.persistence.HolidayDatabase;
 import net.sourceforge.kolmafia.persistence.ItemDatabase;
+import net.sourceforge.kolmafia.preferences.Preferences;
 import net.sourceforge.kolmafia.request.CampgroundRequest;
 import net.sourceforge.kolmafia.request.EquipmentRequest;
 import net.sourceforge.kolmafia.request.GenericRequest;
@@ -224,6 +225,12 @@ public static Cleanups addCampgroundItem(int id) {
     return new Cleanups(() -> CampgroundRequest.removeCampgroundItem(ItemPool.get(id, 1)));
   }
 
+  public static Cleanups setProperty(String key, String value) {
+    var oldValue = Preferences.getString(key);
+    Preferences.setString(key, value);
+    return new Cleanups(() -> Preferences.setString(key, oldValue));
+  }
+
   public static Cleanups setupFakeResponse(int code, String response) {
     GenericRequest.resetClient();
     var builder = new FakeHttpClientBuilder();

test/internal/helpers/RequestLoggerOutput.java

@@ -0,0 +1,20 @@
+package internal.helpers;
+
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
+import net.sourceforge.kolmafia.RequestLogger;
+
+public class RequestLoggerOutput {
+
+  private static ByteArrayOutputStream baos;
+
+  public static void startStream() {
+    baos = new ByteArrayOutputStream();
+    RequestLogger.openCustom(new PrintStream(baos));
+  }
+
+  public static String stopStream() {
+    RequestLogger.closeCustom();
+    return baos.toString();
+  }
+}

test/net/sourceforge/kolmafia/persistence/DebugDatabaseTest.java

@@ -2,6 +2,7 @@
 
 import static org.junit.jupiter.api.Assertions.*;
 
+import internal.helpers.RequestLoggerOutput;
 import java.io.ByteArrayOutputStream;
 import java.io.File;
 import java.io.PrintStream;
@@ -438,13 +439,12 @@ public void checkMeat() {
 
   @Test
   public void itShouldFindSVNDuplicatesSimple() {
-    var outputStream = new ByteArrayOutputStream();
-    RequestLogger.openCustom(new PrintStream(outputStream));
+    RequestLoggerOutput.startStream();
     File svnRoot = mockSimpleSystem();
     DebugDatabase.checkLocalSVNRepository(svnRoot);
     String expected = "Found 1 repo files." + LS;
-    assertEquals(expected, outputStream.toString(), "Output off");
-    RequestLogger.closeCustom();
+    String output = RequestLoggerOutput.stopStream();
+    assertEquals(expected, output, "Output off");
   }
 
   private File mockSimpleSystem() {
@@ -457,13 +457,12 @@ private File mockSimpleSystem() {
 
   @Test
   public void itShouldFindSVNDuplicatesMoreComplex() {
-    var outputStream = new ByteArrayOutputStream();
-    RequestLogger.openCustom(new PrintStream(outputStream));
+    RequestLoggerOutput.startStream();
     File svnRoot = mockMoreComplexSystem();
     DebugDatabase.checkLocalSVNRepository(svnRoot);
     String expected = "Found 3 repo files." + LS;
-    assertEquals(expected, outputStream.toString(), "Output off");
-    RequestLogger.closeCustom();
+    String output = RequestLoggerOutput.stopStream();
+    assertEquals(expected, output, "Output off");
   }
 
   private File mockMoreComplexSystem() {
@@ -488,14 +487,13 @@ private File mockFile(String name) {
 
   @Test
   public void itShouldFindSVNDuplicatesWhenThereAreSome() {
-    var outputStream = new ByteArrayOutputStream();
-    RequestLogger.openCustom(new PrintStream(outputStream));
+    RequestLoggerOutput.startStream();
     File svnRoot = mockDupes();
     DebugDatabase.checkLocalSVNRepository(svnRoot);
     String expected =
         "Found 2 repo files." + LS + "***" + LS + "test.ash" + LS + "test.ash" + LS + "***" + LS;
-    assertEquals(expected, outputStream.toString(), "Output off");
-    RequestLogger.closeCustom();
+    String output = RequestLoggerOutput.stopStream();
+    assertEquals(expected, output, "Output off");
   }
 
   private File mockDupes() {

test/net/sourceforge/kolmafia/textui/command/AbstractCommandTestBase.java

@@ -2,11 +2,9 @@
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 
-import java.io.ByteArrayOutputStream;
-import java.io.PrintStream;
+import internal.helpers.RequestLoggerOutput;
 import net.sourceforge.kolmafia.KoLConstants.MafiaState;
 import net.sourceforge.kolmafia.KoLmafiaCLI;
-import net.sourceforge.kolmafia.RequestLogger;
 import net.sourceforge.kolmafia.StaticEntity;
 
 public abstract class AbstractCommandTestBase {
@@ -17,14 +15,12 @@ public String execute(final String params) {
   }
 
   public String execute(final String params, final boolean check) {
-    var outputStream = new ByteArrayOutputStream();
-    RequestLogger.openCustom(new PrintStream(outputStream));
+    RequestLoggerOutput.startStream();
     var cli = new KoLmafiaCLI(System.in);
     KoLmafiaCLI.isExecutingCheckOnlyCommand = check;
     cli.executeCommand(this.command, params);
     KoLmafiaCLI.isExecutingCheckOnlyCommand = false;
-    RequestLogger.closeCustom();
-    return outputStream.toString();
+    return RequestLoggerOutput.stopStream();
   }
 
   public static void assertState(final MafiaState state) {

test/net/sourceforge/kolmafia/textui/command/ItemTraceCommandTest.java

@@ -0,0 +1,59 @@
+package net.sourceforge.kolmafia.textui.command;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.core.StringContains.containsString;
+
+import internal.helpers.Cleanups;
+import internal.helpers.Player;
+import internal.helpers.RequestLoggerOutput;
+import org.junit.jupiter.api.Test;
+
+public class ItemTraceCommandTest extends AbstractCommandTestBase {
+
+  public ItemTraceCommandTest() {
+    this.command = "itrace";
+  }
+
+  @Test
+  public void doesNotTraceFakeItems() {
+    execute("beefy nigiri");
+
+    RequestLoggerOutput.startStream();
+    Cleanups cleanups = Player.addItem("beefy nigiri");
+    try (cleanups) {
+      var text = RequestLoggerOutput.stopStream();
+      assertThat(text, not(containsString("itrace")));
+    }
+
+    String output = execute("");
+    assertThat(output, containsString("Previously watched items have been cleared"));
+  }
+
+  @Test
+  public void tracesItemsAddedToInventory() {
+    execute("hair spray");
+
+    RequestLoggerOutput.startStream();
+    Cleanups cleanups = Player.addItem("hair spray");
+    try (cleanups) {
+      var text = RequestLoggerOutput.stopStream();
+      assertThat(text, containsString("itrace: hair spray = 1"));
+    }
+  }
+
+  @Test
+  public void clearsPreviousItems() {
+    execute("hair spray");
+    String output = execute("");
+
+    assertThat(output, containsString("Previously watched items have been cleared"));
+
+    RequestLoggerOutput.startStream();
+    Cleanups cleanups = Player.addItem("hair spray");
+    try (cleanups) {
+      var text = RequestLoggerOutput.stopStream();
+      assertThat(text, not(containsString("itrace")));
+    }
+  }
+}