test: Update coverage markers && catch some low-hanging fruit

- Convert some old istanbul markers to c8 markers
- Mark some things as uncoverable that aren't worth testing
- Tweak a couple tests to improve their coverage area, where it's easy
  to do so

src/datastore/kvs/client.test.ts

@@ -120,7 +120,8 @@ class MockServicePort implements Proto.ServicePort<string, string> {
   async request(
     msg: Proto.ClientMsg<string, string>,
   ): Promise<Proto.ServiceMsg<string, string>> {
-    if (!msg) return null;
+    /* c8 ignore next */
+    if (!msg) throw "unreachable";
 
     if (this.inject) return await this.inject(msg);
 

src/datastore/kvs/client.ts

@@ -83,7 +83,8 @@ export default class Client<K extends Proto.Key, V extends Proto.Value>
   private _reconnect() {
     this._port = this.connect(this.name);
     this._port.onNotify = msg => {
-      /* istanbul ignore next */ if (!msg) return;
+      /* c8 ignore next */
+      if (!msg) return;
       switch (msg.$type) {
         case "set":
           this.onSet.send(msg.entries);

src/datastore/kvs/service.test.ts

@@ -31,8 +31,15 @@ describe("datastore/kvs/service", function () {
       svc.onConnect(client);
     });
 
+    afterEach(async () => {
+      svc.onDisconnect(client);
+    });
+
     it("sends updates for new objects", async () => {
-      await svc.set([{key: "a", value: "b"}]);
+      await svc.onRequest(client, {
+        $type: "set",
+        entries: [{key: "a", value: "b"}],
+      });
       await events.next(svc.onSet);
 
       expect(client.received).to.deep.equal([
@@ -41,10 +48,13 @@ describe("datastore/kvs/service", function () {
     });
 
     it("sends updates for multiple new objects at once", async () => {
-      await svc.set([
-        {key: "a", value: "b"},
-        {key: "b", value: "c"},
-      ]);
+      await svc.onRequest(client, {
+        $type: "set",
+        entries: [
+          {key: "a", value: "b"},
+          {key: "b", value: "c"},
+        ],
+      });
       await events.next(svc.onSet);
 
       expect(client.received).to.deep.equal([
@@ -59,8 +69,14 @@ describe("datastore/kvs/service", function () {
     });
 
     it("sends updates for changed objects", async () => {
-      await svc.set([{key: "a", value: "a"}]);
-      await svc.set([{key: "a", value: "alison"}]);
+      await svc.onRequest(client, {
+        $type: "set",
+        entries: [{key: "a", value: "a"}],
+      });
+      await svc.onRequest(client, {
+        $type: "set",
+        entries: [{key: "a", value: "alison"}],
+      });
       await events.nextN(svc.onSet, 2);
 
       expect(client.received).to.deep.equal([
@@ -87,7 +103,7 @@ describe("datastore/kvs/service", function () {
       await svc.set([{key: "b", value: "b"}]);
       await events.nextN(svc.onSet, 2);
 
-      await svc.deleteAll();
+      await svc.onRequest(client, {$type: "deleteAll"});
       await events.next(svc.onSet);
 
       expect(client.received).to.deep.equal([

src/mock/browser/bookmarks.ts

@@ -77,12 +77,12 @@ class MockBookmarks implements BM.Static {
       Math.floor(Math.random() * 3)
     ] as Folder;
 
+    /* c8 ignore next -- bug-checking */
     if (!this.new_bm_parent) throw new Error(`bm startup: no new_bm_parent`);
 
     fixup_child_ordering(this.root);
   }
 
-  /* c8 ignore next 3 -- not implemented */
   async get(idOrIdList: string | string[]): Promise<BM.BookmarkTreeNode[]> {
     if (idOrIdList instanceof Array) {
       return idOrIdList.map(id => node_only(this._get(id)));
@@ -110,6 +110,7 @@ class MockBookmarks implements BM.Static {
   async search(
     query: string | BM.SearchQueryC2Type,
   ): Promise<BM.BookmarkTreeNode[]> {
+    /* c8 ignore next -- not implemented */
     if (typeof query !== "string") throw new Error("Method not implemented.");
 
     const matching: BM.BookmarkTreeNode[] = [];
@@ -280,7 +281,7 @@ class MockBookmarks implements BM.Static {
 
   private _getFolder(id: string): Folder {
     const node = this._get(id);
-    /* c8 ignore next -- bug-checking */
+    /* c8 ignore next 3 -- bug-checking */
     if (!("children" in node)) {
       throw new Error(`${id} is not a folder`);
     }

src/mock/browser/sessions.ts

@@ -21,6 +21,7 @@ class MockSessions implements Sessions.Static {
     this._state = state;
   }
 
+  /* c8 ignore start -- not implemented */
   forgetClosedTab(windowId: number, sessionId: string): Promise<void> {
     throw new Error("Method not implemented.");
   }
@@ -35,6 +36,7 @@ class MockSessions implements Sessions.Static {
   restore(sessionId?: string | undefined): Promise<Sessions.Session> {
     throw new Error("Method not implemented.");
   }
+  /* c8 ignore stop */
 
   async setTabValue(tabId: number, key: string, value: any): Promise<void> {
     const md = this._tab(tabId);

src/mock/browser/tabs-and-windows.ts

@@ -301,15 +301,14 @@ class MockWindows implements W.Static {
     return JSON.parse(JSON.stringify(win));
   }
 
-  /* c8 ignore next -- not used by tests currently */
   async update(
     windowId: number,
     updateInfo: W.UpdateUpdateInfoType,
   ): Promise<W.Window> {
     const win = this._state.win(windowId);
 
     const notImplemented = (name: keyof W.UpdateUpdateInfoType) => {
-      /* c8 ignore next -- not implemented */
+      /* c8 ignore next 3 -- not implemented */
       if (name in updateInfo) {
         throw new Error(`Parameter ${name} is not implemented`);
       }

src/mock/events.ts

@@ -338,9 +338,10 @@ function filter_for(q: EventQuery<any>): (ev: EvMessage) => boolean {
     return _ => true;
   } else if (q instanceof MockEvent) {
     return ev => ev.event === q;
-  } /* c8 ignore next -- bug-checking */ else {
+  } /* c8 ignore start -- bug-checking */ else {
     throw new Error(`Invalid event query: ${inspect(q)}`);
   }
+  /* c8 ignore stop */
 }
 
 /* c8 ignore start -- for manual debugging only */

src/mock/index.ts

@@ -17,6 +17,7 @@ import type {RootHookObject} from "mocha";
     id: "mock",
   },
 };
+/* c8 ignore next 3 -- covering for differences in Node versions */
 if (!(<any>globalThis).navigator) {
   (<any>globalThis).navigator = {hardwareConcurrency: 4};
 }

src/model/deleted-items.ts

@@ -73,13 +73,15 @@ export function findChildItem(
 
   let parent = item;
   for (const index of path.slice(0, path.length - 1)) {
+    /* c8 ignore next 3 -- bug-checking */
     if (!("children" in parent)) {
       throw new Error(`[${path}]: Invalid path in deleted item`);
     }
     parent = parent.children[index];
   }
 
   const index = path[path.length - 1];
+  /* c8 ignore next 3 -- bug-checking */
   if (!("children" in parent) || !parent.children[index]) {
     throw new Error(`[${path}]: Invalid path in deleted item`);
   }

src/model/tabs.ts

@@ -143,12 +143,14 @@ export class Model {
     wiring.listen(browser.tabs.onRemoved, this.whenTabRemoved);
   }
 
+  /* c8 ignore start -- for manual debugging only */
   dumpState(): any {
     return {
       windows: JSON.parse(JSON.stringify(Object.fromEntries(this.windows))),
       tabs: JSON.parse(JSON.stringify(Object.fromEntries(this.tabs))),
     };
   }
+  /* c8 ignore stop */
 
   /** Fetch tabs/windows from the browser again and update the model's
    * understanding of the world with the browser's data.  Use this if it looks
@@ -563,6 +565,7 @@ export class Model {
   whenTabUpdated(id: number, info: Tabs.OnUpdatedChangeInfoType) {
     trace("event tabUpdated", id, info.url, info);
     const t = this.tab(id as TabID);
+    /* c8 ignore start -- compensation for firefox inconsistency */
     if (!t) {
       // Firefox sometimes sends onUpdated events for a tab after it has been
       // closed.  Worse, the usual technique of reloading the model breaks
@@ -576,6 +579,7 @@ export class Model {
       );
       return;
     }
+    /* c8 ignore stop */
 
     if (info.status !== undefined) {
       if (t.status === "loading") --this.loadingCount.value;
@@ -615,11 +619,13 @@ export class Model {
   whenTabMoved(tabId: number, info: {windowId: number; toIndex: number}) {
     trace("event tabMoved", tabId, info);
     const t = this.tab(tabId as TabID);
+    /* c8 ignore next 4 -- compensation for firefox inconsistency */
     if (!t) {
       console.warn(`Got move event for unknown tab ${tabId}`);
       return;
     }
 
+    /* c8 ignore start -- compensation for firefox inconsistency */
     let newWindow = this.window(info.windowId as WindowID);
     if (!newWindow) {
       // Sometimes Firefox sends tabAttached (aka tabMoved) events before
@@ -637,6 +643,7 @@ export class Model {
         alwaysOnTop: false,
       });
     }
+    /* c8 ignore stop */
 
     if (t.position) removeNode(t.position);
     insertNode(t, {parent: newWindow, index: info.toIndex});
@@ -645,6 +652,7 @@ export class Model {
   whenTabReplaced(newId: number, oldId: number) {
     trace("event tabReplaced", oldId, "=>", newId);
     const t = this.tab(oldId as TabID);
+    /* c8 ignore next 4 -- compensation for firefox inconsistency */
     if (!t) {
       console.warn(`Got replace event for unknown tab ${oldId} (-> ${newId})`);
       return;
@@ -658,6 +666,7 @@ export class Model {
   whenTabActivated(info: Tabs.OnActivatedActiveInfoType) {
     trace("event tabActivated", info.tabId, info);
     const tab = this.tab(info.tabId as TabID);
+    /* c8 ignore next 4 -- compensation for firefox inconsistency */
     if (!tab) {
       console.warn(`Got activated event for unknown tab ${info.tabId}`);
       return;
@@ -674,6 +683,7 @@ export class Model {
   whenTabsHighlighted(info: Tabs.OnHighlightedHighlightInfoType) {
     trace("event tabsHighlighted", info);
     const win = this.window(info.windowId as WindowID);
+    /* c8 ignore next 4 -- compensation for firefox inconsistency */
     if (!win) {
       console.log(`Got highlighted event for unknown window ${info.windowId}`);
       return;

src/util/index.ts

@@ -69,15 +69,15 @@ export const bgKeyPressed = (ev: KeyboardEvent | MouseEvent) =>
   PLATFORM_INFO.os === "mac" ? ev.metaKey : ev.ctrlKey;
 /* c8 ignore stop */
 
+/* c8 ignore start -- alt code paths in expect() only happen on bugs */
 /** Checks if its first argument is undefined.  If not, returns it.  If so,
  * throws an error with the message returned by the (optional) second
  * argument. */
 export function expect<T>(value: T | undefined, err: () => string): T {
-  /* c8 ignore start -- if the `throw` is reached, it's a bug */
   if (value !== undefined) return value;
   throw new Error(err());
-  /* c8 ignore stop */
 }
+/* c8 ignore stop */
 
 export const parseVersion = (v: string): number[] =>
   v.split(".").map(x => parseInt(x));
@@ -369,6 +369,7 @@ export type BackingOffOptions = {
   reset_after_idle_ms?: number;
 };
 
+/* c8 ignore start -- alt. config defaults used in testing only */
 const backingOffDefaults: BackingOffOptions = (<any>globalThis).mock?.events
   ? {
       max_delay_ms: 2,
@@ -382,6 +383,7 @@ const backingOffDefaults: BackingOffOptions = (<any>globalThis).mock?.events
       exponent: 0.8,
       reset_after_idle_ms: 300000,
     };
+/* c8 ignore stop */
 
 /** Wraps the passed-in async function so that only one call can be running at a
  * time, and if multiple calls are made within a short time, later calls are

src/util/nanoservice/live.test.ts

@@ -137,8 +137,9 @@ describe("util/nanoservice", function () {
         await events.next(svc.onMessage);
         await events.next("browser.runtime.onMessage");
         await req_p;
-
-        /* c8 ignore next */ throw "unreachable";
+        /* c8 ignore start */
+        throw "unreachable";
+        /* c8 ignore stop */
       } catch (e: any) {
         expect(e).to.be.instanceOf(M.RemoteNanoError);
         expect(e.name).to.equal("Oops");
@@ -241,7 +242,7 @@ describe("util/nanoservice", function () {
             expect(msg.tag).to.equal("b");
             expect(msg.response).to.equal(24);
             break;
-          /* istanbul ignore next */
+          /* c8 ignore next 2 */
           default:
             expect(false).to.be.true;
         }
@@ -287,7 +288,8 @@ describe("util/nanoservice", function () {
     it("drops malformed messages", async function () {
       const [client, svc] = await port_pair("test");
       let called = false;
-      client.onRequest = /* istanbul ignore next */ async () => {
+      /* c8 ignore next 4 */
+      client.onRequest = async () => {
         called = true;
         return null;
       };
@@ -305,7 +307,8 @@ describe("util/nanoservice", function () {
     it("drops responses to requests it didn't send", async () => {
       const [client, svc] = await port_pair("test");
       let called = false;
-      client.onRequest = /* istanbul ignore next */ async () => {
+      /* c8 ignore next 4 */
+      client.onRequest = async () => {
         called = true;
         return null;
       };
@@ -338,7 +341,8 @@ describe("util/nanoservice", function () {
       const p = client
         .request(42)
         .then(
-          /* istanbul ignore next */ () => {
+          /* c8 ignore next 3 */
+          () => {
             throw new Error("Unreachable code");
           },
         )
@@ -380,7 +384,8 @@ describe("util/nanoservice", function () {
     it("ignores connections for other services", async () => {
       let count = 0;
       M.listen("test", {
-        /* istanbul ignore next */ onConnect() {
+        /* c8 ignore next 3 */
+        onConnect() {
           ++count;
         },
       });

src/util/wiring.ts

@@ -26,13 +26,14 @@ export class EventWiring<M> {
       try {
         this.options.onFired();
         return f(...args);
-      } catch (e) /* istanbul ignore next */ {
+      } catch (e) /* c8 ignore start -- bug-checking */ {
         // This is a safety net; unhandled exceptions generally should
         // not happen inside event handlers.
         logError(e);
         this.options.onError(e);
         throw e;
       }
+      /* c8 ignore stop */
     };
 
     ev.addListener(handler);