From 4673e83fa192cd665f891e2d3d7b94db6bec3a6f Mon Sep 17 00:00:00 2001
From: Ben Bartholomew <70723971+ben-bartholomew@users.noreply.github.com>
Date: Thu, 8 Feb 2024 13:59:14 -0500
Subject: [PATCH 1/2] Fixing tests that were being skipped

---
 .../useFeatureOverrides.test.tsx              | 81 +++++++++++--------
 .../FeatureOverrides/useFeatureOverrides.tsx  | 10 +--
 .../src/utils/basic-hooks.ts                  |  2 +-
 3 files changed, 52 insertions(+), 41 deletions(-)

diff --git a/packages/itwin/imodel-react-hooks/src/FeatureOverrides/useFeatureOverrides.test.tsx b/packages/itwin/imodel-react-hooks/src/FeatureOverrides/useFeatureOverrides.test.tsx
index 268b2de74e..2e54aec83b 100644
--- a/packages/itwin/imodel-react-hooks/src/FeatureOverrides/useFeatureOverrides.test.tsx
+++ b/packages/itwin/imodel-react-hooks/src/FeatureOverrides/useFeatureOverrides.test.tsx
@@ -3,46 +3,48 @@
 * See LICENSE.md in the project root for license terms and full copyright notice.
 *--------------------------------------------------------------------------------------------*/
 
-import { IModelApp, Viewport } from "@itwin/core-frontend";
-import { render } from "@testing-library/react";
+import type { FeatureOverrideProvider } from "@itwin/core-frontend";
+import { IModelApp } from "@itwin/core-frontend";
+import { cleanup, render } from "@testing-library/react";
 import React from "react";
-
 import { FeatureOverrideReactProvider, FeatureSymbologyContext, useFeatureOverrides } from "./useFeatureOverrides";
 
 jest.mock("@itwin/core-frontend", () => ({
   IModelApp: {
     viewManager: {
-      __vp: {
-        featureOverrideProvider: {},
-        onFeatureOverrideProviderChanged: {
-          addListener: jest.fn(),
-        },
-        setFeatureOverrideProviderChanged: jest.fn(),
-      },
-      forEachViewport(func: (vp: Viewport) => void) {
-        func((this.__vp as any) as Viewport);
-      },
+      __viewports: [{
+          featureOverrideProviders: new Array<FeatureOverrideProvider>(),
+          onFeatureOverrideProviderChanged: {
+            addListener: jest.fn(),
+          },
+          setFeatureOverrideProviderChanged: jest.fn(),
+          addFeatureOverrideProvider(provider: FeatureOverrideProvider){
+            this.featureOverrideProviders.push(provider);
+          }
+        }
+      ],
       invalidateViewportScenes: jest.fn(),
       onViewOpen: {
         addListener: jest.fn(),
       },
+      [Symbol.iterator]() {
+        return this.__viewports[Symbol.iterator]();
+      }
     },
   },
   FeatureOverrideProvider: class FeatureOverrideProvider { },
 }));
 
-const register = jest.fn();
-const unregister = jest.fn();
-const invalidate = jest.fn();
-
 describe("Hook useFeatureOverrides", () => {
-  beforeEach(() => {
-    register.mockClear();
-    unregister.mockClear();
-    invalidate.mockClear();
-  });
+  afterEach(() => {
+    cleanup();
+  })
 
   it("children are registered in tree order", () => {
+    const unregister = jest.fn();
+    const register = jest.fn();
+    const invalidate = jest.fn();
+
     const a = { overrider: jest.fn() };
     const A = () => {
       useFeatureOverrides(a, []);
@@ -75,25 +77,29 @@ describe("Hook useFeatureOverrides", () => {
     expect(register.mock.calls[2][0].current).toEqual(c);
   });
 
-  it.skip("children unregistered on unmount", () => {
+  it("children unregistered on unmount", async () => {
+
+    const [unregister, unregistered] = getAwaitableMock();
+    const register = jest.fn();
+    const invalidate = jest.fn();
+
     const A = () => {
       useFeatureOverrides({ overrider: jest.fn() }, []);
       return <></>;
     };
     const wrapper = render(
-      <FeatureSymbologyContext.Provider
-        value={{ register, unregister, invalidate }}
-      >
+      <FeatureSymbologyContext.Provider value={{ register, unregister, invalidate }}>
         <A />
-      </FeatureSymbologyContext.Provider>
+      </FeatureSymbologyContext.Provider>,
     );
     expect(register).toBeCalledTimes(1);
     expect(unregister).toBeCalledTimes(0);
     wrapper.unmount();
+    await unregistered;
     expect(unregister).toBeCalledTimes(1);
   });
 
-  it.skip("should ignore components above in the tree when there is a 'complete override'", async () => {
+  it("should ignore components above in the tree when there is a 'complete override'", async () => {
     const override = jest.fn((arg) => {
       const test = arg;
       return test as any;
@@ -112,10 +118,7 @@ describe("Hook useFeatureOverrides", () => {
       return null;
     };
     const C = () => {
-      useFeatureOverrides(
-        { overrider: () => override("C"), completeOverride: true },
-        []
-      );
+      useFeatureOverrides({ overrider: () => override("C"), completeOverride: true }, []);
       return <D />;
     };
     const D = () => {
@@ -125,12 +128,22 @@ describe("Hook useFeatureOverrides", () => {
     render(
       <FeatureOverrideReactProvider>
         <A />
-      </FeatureOverrideReactProvider>
+      </FeatureOverrideReactProvider>,
     );
     await new Promise((resolve) => setTimeout(resolve, 10));
     for (const vp of IModelApp.viewManager) {
-      vp.addFeatureOverrideProvider(undefined!);
+      for (const provider of vp.featureOverrideProviders) {
+        provider.addFeatureOverrides(undefined!, vp);
+      }
     }
     expect(override.mock.calls).toEqual([["C"], ["D"]]);
   });
 });
+
+function getAwaitableMock(): [jest.Mock<void>, Promise<void>] {
+  let resolve: (() => void) | undefined;
+  const promise = new Promise<void>((_resolve) => {resolve = _resolve});
+  const callback = jest.fn(resolve);
+
+  return [callback, promise];
+}
diff --git a/packages/itwin/imodel-react-hooks/src/FeatureOverrides/useFeatureOverrides.tsx b/packages/itwin/imodel-react-hooks/src/FeatureOverrides/useFeatureOverrides.tsx
index 494353a6c0..362e4e3c05 100644
--- a/packages/itwin/imodel-react-hooks/src/FeatureOverrides/useFeatureOverrides.tsx
+++ b/packages/itwin/imodel-react-hooks/src/FeatureOverrides/useFeatureOverrides.tsx
@@ -4,10 +4,8 @@
 *--------------------------------------------------------------------------------------------*/
 
 import type { FeatureOverrideProvider, FeatureSymbology, Viewport } from "@itwin/core-frontend";
+import React, { useCallback, useContext, useEffect, useMemo, useRef } from "react";
 import { IModelApp } from "@itwin/core-frontend";
-import React, { useContext, useEffect, useMemo, useRef } from "react";
-import { useCallback } from "react";
-
 import { useOnMountInRenderOrder } from "../utils/basic-hooks";
 import { makeContextWithProviderRequired } from "../utils/react-context";
 
@@ -116,14 +114,14 @@ export const FeatureOverrideReactProvider = ({
     };
     attach();
 
-    IModelApp.viewManager.onViewOpen.addListener(attach);
+    const removeListener = IModelApp.viewManager.onViewOpen.addListener(attach);
     return () => {
       for (const vp of IModelApp.viewManager) {
         if (!viewFilter || viewFilter(vp)) {
           vp.dropFeatureOverrideProvider(impl);
         }
       }
-      IModelApp.viewManager.onViewOpen.removeListener(attach);
+      removeListener();
     };
   }, [impl, viewFilter]);
 
@@ -147,7 +145,7 @@ export const FeatureOverrideReactProvider = ({
       },
       invalidate,
     }),
-    [providers, viewFilter]
+    [providers, invalidate]
   );
 
   return (
diff --git a/packages/itwin/imodel-react-hooks/src/utils/basic-hooks.ts b/packages/itwin/imodel-react-hooks/src/utils/basic-hooks.ts
index 2e802e3826..aeccbafe2a 100644
--- a/packages/itwin/imodel-react-hooks/src/utils/basic-hooks.ts
+++ b/packages/itwin/imodel-react-hooks/src/utils/basic-hooks.ts
@@ -3,7 +3,7 @@
 * See LICENSE.md in the project root for license terms and full copyright notice.
 *--------------------------------------------------------------------------------------------*/
 
-import { useEffect, useMemo, useRef } from "react";
+import { useEffect, useRef } from "react";
 
 /** perform code on mount, executing in render order, which
  * effects cannot do.

From cfac6169a949fd23183ba465e48976a71fb4d1b6 Mon Sep 17 00:00:00 2001
From: Ben Bartholomew <70723971+ben-bartholomew@users.noreply.github.com>
Date: Thu, 8 Feb 2024 14:09:02 -0500
Subject: [PATCH 2/2] Creating changelog patch file

---
 ...l-react-hooks-559d8d84-e1ad-466b-828f-993f7cef0338.json | 7 +++++++
 1 file changed, 7 insertions(+)
 create mode 100644 change/@itwin-imodel-react-hooks-559d8d84-e1ad-466b-828f-993f7cef0338.json

diff --git a/change/@itwin-imodel-react-hooks-559d8d84-e1ad-466b-828f-993f7cef0338.json b/change/@itwin-imodel-react-hooks-559d8d84-e1ad-466b-828f-993f7cef0338.json
new file mode 100644
index 0000000000..d3ea910c97
--- /dev/null
+++ b/change/@itwin-imodel-react-hooks-559d8d84-e1ad-466b-828f-993f7cef0338.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Fixing tests for imodel-react-hooks that were being skipped",
+  "packageName": "@itwin/imodel-react-hooks",
+  "email": "70723971+ben-bartholomew@users.noreply.github.com",
+  "dependentChangeType": "patch"
+}