[arvados-workbench2] updated: 2.6.3-36-g5430c336

git repository hosting git at public.arvados.org
Wed Aug 2 00:45:22 UTC 2023


Summary of changes:
 package.json                                       |  1 +
 src/common/use-async-interval.test.tsx             | 96 ++++++++++++++++++++++
 src/common/use-async-interval.ts                   | 52 +++++++-----
 .../process-logs-panel-actions.ts                  |  3 +-
 yarn.lock                                          | 19 +++++
 5 files changed, 149 insertions(+), 22 deletions(-)
 create mode 100644 src/common/use-async-interval.test.tsx

       via  5430c336b96cbb7c20bffa1cbdb8cffea32fb460 (commit)
      from  d76aa6ba31074f3d41cef48628b235aab4906ce5 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.


commit 5430c336b96cbb7c20bffa1cbdb8cffea32fb460
Author: Stephen Smith <stephen at curii.com>
Date:   Tue Aug 1 20:40:13 2023 -0400

    20219: Improve useAsyncInterval testability, add unit test
    
    * Combines useRefs for easier mocking
    * Wraps callback execution in promise chain to prevent halting on non-async
      callbacks
    * Clean up now unused async contexts
    * Add explicit promise resolve/reject on PollProcessLogs
      * Add catch block to continue polling on error
    
    Arvados-DCO-1.1-Signed-off-by: Stephen Smith <stephen at curii.com>

diff --git a/package.json b/package.json
index 0a10da78..5f1f647d 100644
--- a/package.json
+++ b/package.json
@@ -9,6 +9,7 @@
     "@fortawesome/react-fontawesome": "0.1.9",
     "@material-ui/core": "3.9.3",
     "@material-ui/icons": "3.0.1",
+    "@sinonjs/fake-timers": "^10.3.0",
     "@types/debounce": "3.0.0",
     "@types/file-saver": "2.0.0",
     "@types/js-yaml": "3.11.2",
diff --git a/src/common/use-async-interval.test.tsx b/src/common/use-async-interval.test.tsx
new file mode 100644
index 00000000..188f184b
--- /dev/null
+++ b/src/common/use-async-interval.test.tsx
@@ -0,0 +1,96 @@
+// Copyright (C) The Arvados Authors. All rights reserved.
+//
+// SPDX-License-Identifier: AGPL-3.0
+
+import React from 'react';
+import { useAsyncInterval } from './use-async-interval';
+import { configure, mount } from 'enzyme';
+import Adapter from 'enzyme-adapter-react-16';
+import FakeTimers from "@sinonjs/fake-timers";
+
+configure({ adapter: new Adapter() });
+const clock = FakeTimers.install();
+
+jest.mock('react', () => {
+    const originalReact = jest.requireActual('react');
+    const mUseRef = jest.fn();
+    return {
+        ...originalReact,
+        useRef: mUseRef,
+    };
+});
+
+const TestComponent = (props): JSX.Element => {
+    useAsyncInterval(props.callback, 2000);
+    return <span />;
+};
+
+describe('useAsyncInterval', () => {
+    it('should fire repeatedly after the interval', async () => {
+        const mockedReact = React as jest.Mocked<typeof React>;
+        const ref = { current: {} };
+        mockedReact.useRef.mockReturnValue(ref);
+
+        const syncCallback = jest.fn();
+        const testComponent = mount(<TestComponent
+            callback={syncCallback}
+        />);
+
+        // cb queued with interval but not called
+        expect(syncCallback).not.toHaveBeenCalled();
+
+        // wait for first tick
+        await clock.tickAsync(2000);
+        expect(syncCallback).toHaveBeenCalledTimes(1);
+
+        // wait for second tick
+        await clock.tickAsync(2000);
+        expect(syncCallback).toHaveBeenCalledTimes(2);
+
+        // wait for third tick
+        await clock.tickAsync(2000);
+        expect(syncCallback).toHaveBeenCalledTimes(3);
+    });
+
+    it('should wait for async callbacks to complete in between polling', async () => {
+        const mockedReact = React as jest.Mocked<typeof React>;
+        const ref = { current: {} };
+        mockedReact.useRef.mockReturnValue(ref);
+
+        const delayedCallback = jest.fn(() => (
+            new Promise<void>((resolve) => {
+                setTimeout(() => {
+                    resolve();
+                }, 2000);
+            })
+        ));
+        const testComponent = mount(<TestComponent
+            callback={delayedCallback}
+        />);
+
+        // cb queued with setInterval but not called
+        expect(delayedCallback).not.toHaveBeenCalled();
+
+        // Wait 2 seconds for first tick
+        await clock.tickAsync(2000);
+        // First cb called after 2 seconds
+        expect(delayedCallback).toHaveBeenCalledTimes(1);
+        // Wait for cb to resolve for 2 seconds
+        await clock.tickAsync(2000);
+        expect(delayedCallback).toHaveBeenCalledTimes(1);
+
+        // Wait 2 seconds for second tick
+        await clock.tickAsync(2000);
+        expect(delayedCallback).toHaveBeenCalledTimes(2);
+        // Wait for cb to resolve for 2 seconds
+        await clock.tickAsync(2000);
+        expect(delayedCallback).toHaveBeenCalledTimes(2);
+
+        // Wait 2 seconds for third tick
+        await clock.tickAsync(2000);
+        expect(delayedCallback).toHaveBeenCalledTimes(3);
+        // Wait for cb to resolve for 2 seconds
+        await clock.tickAsync(2000);
+        expect(delayedCallback).toHaveBeenCalledTimes(3);
+    });
+});
diff --git a/src/common/use-async-interval.ts b/src/common/use-async-interval.ts
index 8951a9b0..3be7309a 100644
--- a/src/common/use-async-interval.ts
+++ b/src/common/use-async-interval.ts
@@ -2,34 +2,44 @@
 //
 // SPDX-License-Identifier: AGPL-3.0
 
-var react = require("react");
+import React from "react";
 
 export const useAsyncInterval = function (callback, delay) {
-    const savedCallback = react.useRef();
-    const active = react.useRef(false);
+    const ref = React.useRef<{cb: () => Promise<any>, active: boolean}>({
+        cb: async () => {},
+        active: false}
+    );
 
     // Remember the latest callback.
-    react.useEffect(() => {
-        savedCallback.current = callback;
+    React.useEffect(() => {
+        ref.current.cb = callback;
     }, [callback]);
     // Set up the interval.
-    react.useEffect(() => {
-        // useEffect doesn't like async callbacks (https://github.com/facebook/react/issues/14326) so create nested async callback
-        (async () => {
-            // Make tick() async
-            async function tick() {
-                if (active.current) {
-                    // If savedCallback is not set yet, no-op until it is
-                    savedCallback.current && await savedCallback.current();
+    React.useEffect(() => {
+        function tick() {
+            if (ref.current.active) {
+                // Wrap execution chain with promise so that execution errors or
+                //   non-async callbacks still fall through to .finally, avoids breaking polling
+                new Promise((resolve) => {
+                    return resolve(ref.current.cb());
+                }).then(() => {
+                    // Promise succeeded
+                    // Possibly implement back-off reset
+                }).catch(() => {
+                    // Promise rejected
+                    // Possibly implement back-off in the future
+                }).finally(() => {
                     setTimeout(tick, delay);
-                }
+                });
             }
-            if (delay !== null) {
-                active.current = true;
-                setTimeout(tick, delay);
-            }
-        })(); // Call nested async function
-        // We return the teardown function here since we can't from inside the nested async callback
-        return () => {active.current = false;};
+        }
+        if (delay !== null) {
+            ref.current.active = true;
+            setTimeout(tick, delay);
+        }
+        // Suppress warning about cleanup function - can be ignored when variables are unrelated to dom elements
+        //   https://github.com/facebook/react/issues/15841#issuecomment-500133759
+        // eslint-disable-next-line
+        return () => {ref.current.active = false;};
     }, [delay]);
 };
diff --git a/src/store/process-logs-panel/process-logs-panel-actions.ts b/src/store/process-logs-panel/process-logs-panel-actions.ts
index a0d071fd..328ca4da 100644
--- a/src/store/process-logs-panel/process-logs-panel-actions.ts
+++ b/src/store/process-logs-panel/process-logs-panel-actions.ts
@@ -92,8 +92,9 @@ export const pollProcessLogs = (processUuid: string) =>
                     await dispatch(processLogsPanelActions.ADD_PROCESS_LOGS_PANEL_ITEM(groupedLogs));
                 }
             }
+            return Promise.resolve();
         } catch (e) {
-            // Failed to poll, ignore error
+            return Promise.reject();
         }
     };
 
diff --git a/yarn.lock b/yarn.lock
index 20bbc46b..826baac8 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -2233,6 +2233,24 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@sinonjs/commons at npm:^3.0.0":
+  version: 3.0.0
+  resolution: "@sinonjs/commons at npm:3.0.0"
+  dependencies:
+    type-detect: 4.0.8
+  checksum: b4b5b73d4df4560fb8c0c7b38c7ad4aeabedd362f3373859d804c988c725889cde33550e4bcc7cd316a30f5152a2d1d43db71b6d0c38f5feef71fd8d016763f8
+  languageName: node
+  linkType: hard
+
+"@sinonjs/fake-timers at npm:^10.3.0":
+  version: 10.3.0
+  resolution: "@sinonjs/fake-timers at npm:10.3.0"
+  dependencies:
+    "@sinonjs/commons": ^3.0.0
+  checksum: 614d30cb4d5201550c940945d44c9e0b6d64a888ff2cd5b357f95ad6721070d6b8839cd10e15b76bf5e14af0bcc1d8f9ec00d49a46318f1f669a4bec1d7f3148
+  languageName: node
+  linkType: hard
+
 "@sinonjs/formatio at npm:^3.2.1":
   version: 3.2.2
   resolution: "@sinonjs/formatio at npm:3.2.2"
@@ -3729,6 +3747,7 @@ __metadata:
     "@fortawesome/react-fontawesome": 0.1.9
     "@material-ui/core": 3.9.3
     "@material-ui/icons": 3.0.1
+    "@sinonjs/fake-timers": ^10.3.0
     "@types/classnames": 2.2.6
     "@types/debounce": 3.0.0
     "@types/enzyme": 3.1.14

-----------------------------------------------------------------------


hooks/post-receive
-- 




More information about the arvados-commits mailing list