fix(usePersistedStateReducer.test): Fix flaky test of push tab retry (#…

…2191) * test(usePersistedStateReducerTest): wait for all promises to resolve before assertions * refactor(usePersistedStateReducer): store remaining retries with ref instead of state Prevents possible race conditions with between asynchronously updating the remaining retries count state & updating the tab state from dispatching actions * test(usePersistedStateReducer): await act in other retry logic test
mbta · Aug 22, 2023 · dddf906 · dddf906
1 parent debeeb3
commit dddf906
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 32 deletions.
diff --git a/assets/src/hooks/usePersistedStateReducer.ts b/assets/src/hooks/usePersistedStateReducer.ts
@@ -1,4 +1,4 @@
-import { useEffect, useReducer, useState } from "react"
+import { useEffect, useReducer, useRef } from "react"
 import { putRouteTabs } from "../api"
 import appData from "../appData"
 import { loadState, saveState } from "../localStorage"
@@ -48,32 +48,31 @@ const usePersistedStateReducer = (): [State, Dispatch] => {
 
   const { routeTabsToPush, routeTabsPushInProgress } = state
 
-  const [routeTabsPushRetriesLeft, setRouteTabsPushRetriesLeft] =
-    useState(routeTabsPushRetries)
+  const routeTabsPushRetriesLeft = useRef(routeTabsPushRetries)
 
-  /* eslint-disable react-hooks/exhaustive-deps */
   useEffect(() => {
     if (routeTabsToPush && !routeTabsPushInProgress) {
+      const currentTriesLeft = routeTabsPushRetriesLeft.current
       dispatch(startingRouteTabsPush())
       putRouteTabs(routeTabsToPush)
         .then((response) => {
           if (response.ok) {
-            setRouteTabsPushRetriesLeft(routeTabsPushRetries)
+            routeTabsPushRetriesLeft.current = routeTabsPushRetries
             dispatch(routeTabsPushComplete())
-          } else if (routeTabsPushRetriesLeft > 0) {
-            setRouteTabsPushRetriesLeft(routeTabsPushRetriesLeft - 1)
+          } else if (currentTriesLeft > 0) {
+            routeTabsPushRetriesLeft.current = currentTriesLeft - 1
             dispatch(retryRouteTabsPushIfNotOutdated(routeTabsToPush))
           } else {
-            setRouteTabsPushRetriesLeft(routeTabsPushRetries)
+            routeTabsPushRetriesLeft.current = routeTabsPushRetries
             dispatch(routeTabsPushComplete())
           }
         })
         .catch(() => {
-          if (routeTabsPushRetriesLeft > 0) {
-            setRouteTabsPushRetriesLeft(routeTabsPushRetriesLeft - 1)
+          if (currentTriesLeft > 0) {
+            routeTabsPushRetriesLeft.current = currentTriesLeft - 1
             dispatch(retryRouteTabsPushIfNotOutdated(routeTabsToPush))
           } else {
-            setRouteTabsPushRetriesLeft(routeTabsPushRetries)
+            routeTabsPushRetriesLeft.current = routeTabsPushRetries
             dispatch(routeTabsPushComplete())
           }
         })

diff --git a/assets/tests/hooks/usePersistedStateReducer.test.ts b/assets/tests/hooks/usePersistedStateReducer.test.ts
@@ -6,7 +6,7 @@ import {
   beforeEach,
   afterAll,
 } from "@jest/globals"
-import { act, renderHook, waitFor } from "@testing-library/react"
+import { act, renderHook } from "@testing-library/react"
 import { putUserSetting, putRouteTabs } from "../../src/api"
 import appData from "../../src/appData"
 import usePersistedStateReducer, {
@@ -320,24 +320,13 @@ describe("usePersistedStateReducer", () => {
       .mockImplementationOnce(() => fakePromise)
       .mockImplementationOnce(() => fakePromise)
 
-    const firstValue = result.current
-
-    act(() => {
+    await act(async () => {
       dispatch(createRouteTab())
     })
-    await waitFor(() => {
-      expect(result.current).not.toBe(firstValue)
-    })
-
-    // wait for changes from dispatch call in catch handler
-    const secondValue = result.current
-
-    await waitFor(() => {
-      expect(result.current).not.toBe(secondValue)
-    })
 
     const [state] = result.current
 
+    expect(putRouteTabs).toHaveBeenCalledTimes(3)
     expect(state.routeTabs).toMatchObject([
       {
         ordering: 0,
@@ -347,7 +336,6 @@ describe("usePersistedStateReducer", () => {
     expect(state.routeTabsToPush).toBeNull()
     expect(state.routeTabsToPushNext).toBeNull()
     expect(state.routeTabsPushInProgress).toEqual(false)
-    expect(putRouteTabs).toHaveBeenCalledTimes(3)
   })
 
   test("retries at most two more times, with final failure being a client error", async () => {
@@ -363,16 +351,13 @@ describe("usePersistedStateReducer", () => {
       .mockImplementationOnce(() => fakePromise)
       .mockImplementationOnce(() => fakePromise)
 
-    act(() => {
+    await act(async () => {
       dispatch(createRouteTab())
     })
-    const initialValue = result.current
-    await waitFor(() => {
-      expect(result.current).not.toBe(initialValue)
-    })
 
     const [state] = result.current
 
+    expect(putRouteTabs).toHaveBeenCalledTimes(3)
     expect(state.routeTabs).toMatchObject([
       {
         ordering: 0,
@@ -382,7 +367,6 @@ describe("usePersistedStateReducer", () => {
     expect(state.routeTabsToPush).toBeNull()
     expect(state.routeTabsToPushNext).toBeNull()
     expect(state.routeTabsPushInProgress).toEqual(false)
-    expect(putRouteTabs).toHaveBeenCalledTimes(3)
   })
 })