ni / nimble

The NI Nimble Design System
https://nimble.ni.dev
MIT License
29 stars 9 forks source link

Non-fakeAsync tests leak update tasks, break waitForUpdatesAsync #1933

Open TrevorKarjanis opened 3 months ago

TrevorKarjanis commented 3 months ago

πŸ› Bug Report

Tests timeout in a specific test suite, but not others, because waitForUpdatesAsync hangs. waitForUpdatesAsync hangs, because fast-element@1.12.0 DOM.nextUpdate adds tasks but does not process tasks if they already exist. All subsequent tests that use waitForUpdatesAsync fail.

The Nimble documentation states that processUpdates should be called in afterEach for fakeAsync tests but not waitForAsync tests. There are other waitForAsync test suites in the same workspace that use waitForUpdatesAsync and ComponentFixture.whenStable that do not fail. waitForUpdatesAsync seems to only be needed for certain components, like menus and selects. Otherwise, whenStable updates Nimble components without issue.

πŸ’» Repro or Code Sample

Replace processUpdates with waitForUpdatesAsync in the workspace members test suite. The original commit was 199bf0. See Current Behavior for the error logs.

πŸ€” Expected Behavior

Either calling processUpdates from afterEach should be required for asynchronous tests or waitForUpdatesAsync should process tasks when tasks already exist. The former expectation is problematic, because it is not a requirement of many tests.

😯 Current Behavior

The test and subsequent tests that use waitForUpdatesAsync timeout.

Chrome Headless 123.0.6312.4 (Linux x86_64) WorkspaceEditorComponent Permissions Member mapping updates not allowed Should disable role selectors FAILED
    Error: Timeout - Async function did not complete within 10000ms (set by jasmine.DEFAULT_TIMEOUT_INTERVAL)
        at <Jasmine>
Chrome Headless 123.0.6312.4 (Linux x86_64): Executed 146 of 239 (3 FAILED) (0 secs / 32.299 secs)
Chrome Headless 123.0.6312.4 (Linux x86_64) WorkspaceEditorComponent Permissions Member mapping updates not allowed Should disable role selectors FAILED
    Error: Timeout - Async function did not complete within 10000ms (set by jasmine.DEFAULT_TIMEOUT_INTERVAL)
        at <Jasmine>

πŸ’ Possible Solution

πŸ”¦ Context

I migrated the security application to use Nimble components. The view whose test suite was affected by this issue includes nimble-button, nimble-icon-*, and nimble-menu-button. It used to include nimble-select.

🌍 Your Environment

TrevorKarjanis commented 3 months ago

I took an innovation sprint to investigate this and other related asynchronous testing issues. I created a repository, Angular Asynchronous Test Specification, which contains a test specification, async.spec.ts, of various asynchronous testing scenarios including micro and macrotasks, RxJS, and Nimble components. I reproduced and cataloged the issue described in this bug report and a few others which I have copied to outline here. In conclusion, I updated the async testing guidelines.

  • fakeAsync may not work with even simple asynchronous Nimble updates like button appearance.
  • Nimble's waitForUpdatesAsync has two common issues.
    • In watch mode, waitForUpdatesAsync may timeout after a change and compile. Refresh the page to rerun the test.
    • non-fakeAsync tests can leak tasks that break calls to waitForUpdatesAsync for all subsequent tests.
  • There have been a few reported cases where ComponentFixture.whenStable did not work and waitForUpdatesAsync was required for components with a menu and select. However, the exact reason is not known.