keep the health checks running after a collection install

src/renderer/src/extensions/collections/util/InstallDriver.test.ts

@@ -456,6 +456,96 @@ describe("InstallDriver completion decision", () => {
   });
 });
 
+describe("InstallDriver health-check suppression", () => {
+  test("releases the suppressed health checks once the finished install is closed", async ({
+    makeDriver,
+  }) => {
+    const h = makeDriver({
+      mods: { [GAME_ID]: { [defaultFixture.collection.id]: defaultFixture.collection } },
+      downloads: { [defaultFixture.download.id]: defaultFixture.download },
+      profiles: { [profile.id]: profile },
+    });
+    let released = false;
+    Object.assign(h.api.ext, {
+      withSuppressedTests: (_tests: string[], cb: () => PromiseLike<void>) =>
+        cb().then(() => {
+          released = true;
+        }),
+    });
+    await h.driver.start(profile, defaultFixture.collection);
+    setSessionStatus(h, [memberRule], "installed");
+    h.emit("did-install-dependencies", GAME_ID, defaultFixture.collection.id, false);
+    await vi.waitFor(() => expect(h.driver.step).toBe("review"));
+
+    await h.driver.continue();
+
+    await vi.waitFor(() => expect(released).toBe(true));
+  });
+
+  test("holds one suppression when an unfinished install is started again", async ({
+    makeDriver,
+  }) => {
+    const h = makeDriver({
+      mods: { [GAME_ID]: { [defaultFixture.collection.id]: defaultFixture.collection } },
+      downloads: { [defaultFixture.download.id]: defaultFixture.download },
+      profiles: { [profile.id]: profile },
+    });
+    let held = 0;
+    Object.assign(h.api.ext, {
+      withSuppressedTests: (_tests: string[], cb: () => PromiseLike<void>) => {
+        held += 1;
+        return cb().then(() => {
+          held -= 1;
+        });
+      },
+    });
+    await h.driver.start(profile, defaultFixture.collection);
+    // the member stays pending, so the install can be started again
+    h.emit("did-install-dependencies", GAME_ID, defaultFixture.collection.id, false);
+    await vi.waitFor(() => expect(h.driver.installDone).toBe(true));
+
+    await h.driver.start(profile, defaultFixture.collection);
+
+    await vi.waitFor(() => expect(held).toBe(1));
+  });
+
+  test("releases the suppression when the game-version prompt is cancelled", async ({
+    makeDriver,
+  }) => {
+    // game versions on the download, read without a fetch
+    const download = makeDownload({
+      ...defaultFixture.download,
+      modInfo: {
+        ...defaultFixture.download.modInfo,
+        nexus: {
+          ...defaultFixture.download.modInfo.nexus,
+          revisionInfo: { modFiles: [], gameVersions: [{ reference: "9.9.9" }] },
+        },
+      },
+    });
+    const h = makeDriver({
+      mods: { [GAME_ID]: { [defaultFixture.collection.id]: defaultFixture.collection } },
+      downloads: { [download.id]: download },
+      profiles: { [profile.id]: profile },
+    });
+    let held = 0;
+    Object.assign(h.api.ext, {
+      withSuppressedTests: (_tests: string[], cb: () => PromiseLike<void>) => {
+        held += 1;
+        return cb().then(() => {
+          held -= 1;
+        });
+      },
+    });
+    h.setNextDialog({ action: "Cancel", input: {} });
+
+    await h.driver.start(profile, defaultFixture.collection);
+
+    expect(h.dialogCalls.map((call) => call.title)).toContain("Game version mismatch");
+    await vi.waitFor(() => expect(held).toBe(0));
+  });
+});
+
 describe("InstallDriver installRecommended", () => {
   // "Install optional mods" (dialog) clears the durable skip on the optional members and re-runs the
   // NORMAL collection dependency install (the same install-dependencies event begin() uses) rather

src/renderer/src/extensions/collections/util/InstallDriver.ts

@@ -796,20 +796,30 @@ class InstallDriver {
   }
 
   private startInstall = async () => {
-    // suppress plugins-changed event to avoid constantly running expensive callbacks
-    // until onStop gets called
+    // a restarted install replaces the hold it already has
+    this.mOnStop?.();
+    // hold off the checks while the collection installs
     this.mApi.ext.withSuppressedTests?.(
       ["plugins-changed", "settings-changed", "mod-activated", "mod-installed"],
       () =>
-        new Bluebird((resolve) => {
+        new Promise<void>((resolve) => {
           this.mOnStop = () => {
-            resolve(undefined);
+            resolve();
             this.mOnStop = undefined;
           };
         }),
     );
 
-    return this.startImpl();
+    // release when the install never starts
+    let started: boolean | undefined = false;
+    try {
+      started = await this.startImpl();
+      return started;
+    } finally {
+      if (started === false) {
+        this.mOnStop?.();
+      }
+    }
   };
 
   private startImpl = async () => {
@@ -1105,6 +1115,8 @@ class InstallDriver {
     this.mCollection = undefined;
     this.setDependentMods([]);
     this.mInstallDone = true;
+    // release the checks the install held off
+    this.mOnStop?.();
     this.triggerUpdate();
   };
 

src/renderer/src/extensions/test_runner/index.ts

@@ -47,6 +47,7 @@ import { log } from "../../util/log";
 import { activeGameId, activeProfile } from "../../util/selectors";
 import { getSafe } from "../../util/storeHelper";
 import { setdefault } from "../../util/util";
+import { skipWhileSuppressed, withSuppressedTests } from "./suppressedTests";
 
 interface ICheckEntry {
   id: string;
@@ -182,10 +183,8 @@ function runCheck(api: IExtensionApi, check: ICheckEntry): Promise<void> {
     });
 }
 
-const suppressedTests: { [testId: string]: number } = {};
-
 function runChecks(api: IExtensionApi, event: string, delay?: number) {
-  if (suppressedTests[event] ?? 0 > 0) {
+  if (skipWhileSuppressed(event, () => runChecks(api, event, delay))) {
     return;
   }
 
@@ -202,19 +201,6 @@ function runChecks(api: IExtensionApi, event: string, delay?: number) {
   }, delay || 500);
 }
 
-function withSuppressedTests(tests: string[], cb: () => PromiseBB<void>) {
-  tests.forEach((test) => {
-    setdefault(suppressedTests, test, 0);
-    suppressedTests[test] += 1;
-  });
-
-  return cb().finally(() => {
-    tests.forEach((test) => {
-      suppressedTests[test] -= 1;
-    });
-  });
-}
-
 function init(context: IExtensionContext): boolean {
   context.registerTest = (id, eventType, check) => {
     log("debug", "register test", { id, eventType });

src/renderer/src/extensions/test_runner/suppressedTests.test.ts

@@ -0,0 +1,54 @@
+import { describe, expect, it, vi } from "vitest";
+
+import { skipWhileSuppressed, withSuppressedTests } from "./suppressedTests";
+
+// a suppression held until the returned release is called
+function hold(tests: string[]): { released: Promise<void>; release: () => void } {
+  let release: () => void = () => undefined;
+  const released = withSuppressedTests(
+    tests,
+    () =>
+      new Promise<void>((resolve) => {
+        release = resolve;
+      }),
+  );
+  return { released, release };
+}
+
+describe("withSuppressedTests", () => {
+  it("runs a check triggered while suppressed once the suppression lifts", async () => {
+    const { released, release } = hold(["plugins-changed"]);
+    const run = vi.fn();
+
+    expect(skipWhileSuppressed("plugins-changed", run)).toBe(true);
+    expect(run).not.toHaveBeenCalled();
+
+    release();
+    await released;
+
+    expect(run).toHaveBeenCalledTimes(1);
+  });
+
+  it("runs nothing on release when no check was triggered", async () => {
+    const { released, release } = hold(["mod-installed"]);
+    release();
+    await released;
+
+    expect(skipWhileSuppressed("mod-installed", vi.fn())).toBe(false);
+  });
+
+  it("runs the skipped check only when the last overlapping suppression lifts", async () => {
+    const outer = hold(["settings-changed"]);
+    const inner = hold(["settings-changed"]);
+    const run = vi.fn();
+    skipWhileSuppressed("settings-changed", run);
+
+    inner.release();
+    await inner.released;
+    expect(run).not.toHaveBeenCalled();
+
+    outer.release();
+    await outer.released;
+    expect(run).toHaveBeenCalledTimes(1);
+  });
+});

src/renderer/src/extensions/test_runner/suppressedTests.ts

@@ -0,0 +1,33 @@
+// active suppressions, keyed by test event type
+const suppressedTests: Record<string, number> = {};
+// the latest run a suppression skipped, keyed by test event type
+const skippedRuns: Record<string, () => void> = {};
+
+/**
+ * Whether checks for the event are suppressed. A run skipped while suppressed is replayed once the
+ * last suppression lifts.
+ */
+export function skipWhileSuppressed(event: string, run: () => void): boolean {
+  if ((suppressedTests[event] ?? 0) === 0) {
+    return false;
+  }
+  skippedRuns[event] = run;
+  return true;
+}
+
+export function withSuppressedTests(tests: string[], cb: () => PromiseLike<void>) {
+  tests.forEach((test) => {
+    suppressedTests[test] = (suppressedTests[test] ?? 0) + 1;
+  });
+
+  return Promise.resolve(cb()).finally(() => {
+    tests.forEach((test) => {
+      suppressedTests[test] -= 1;
+      const skipped = skippedRuns[test];
+      if (suppressedTests[test] === 0 && skipped !== undefined) {
+        delete skippedRuns[test];
+        skipped();
+      }
+    });
+  });
+}