From 3ab48962344982f02c93c5de5c5dcde849ef6461 Mon Sep 17 00:00:00 2001 From: timdeschryver <28659384+timdeschryver@users.noreply.github.com> Date: Fri, 25 Jun 2021 08:36:14 +0200 Subject: [PATCH 1/2] fix: infite loop in waitFor when DOM is mutating --- .../examples/13-scrolling.component.spec.ts | 2 +- .../src/app/issues/issue-230.spec.ts | 23 +++++++++++++++++++ .../src/lib/testing-library.ts | 13 ++++++++--- .../testing-library/tests/wait-for.spec.ts | 6 ++--- 4 files changed, 37 insertions(+), 7 deletions(-) create mode 100644 apps/example-app/src/app/issues/issue-230.spec.ts diff --git a/apps/example-app/src/app/examples/13-scrolling.component.spec.ts b/apps/example-app/src/app/examples/13-scrolling.component.spec.ts index 2126e255..4abaed08 100644 --- a/apps/example-app/src/app/examples/13-scrolling.component.spec.ts +++ b/apps/example-app/src/app/examples/13-scrolling.component.spec.ts @@ -12,7 +12,7 @@ test('should scroll to load more items', async () => { expect(item0).toBeVisible(); screen.getByTestId('scroll-viewport').scrollTop = 500; - await waitForElementToBeRemoved(() => screen.getByText(/Item #0/i)); + await waitForElementToBeRemoved(() => screen.queryByText(/Item #0/i)); const item12 = await screen.findByText(/Item #12/i); expect(item12).toBeVisible(); diff --git a/apps/example-app/src/app/issues/issue-230.spec.ts b/apps/example-app/src/app/issues/issue-230.spec.ts new file mode 100644 index 00000000..3ac54147 --- /dev/null +++ b/apps/example-app/src/app/issues/issue-230.spec.ts @@ -0,0 +1,23 @@ +import { Component } from '@angular/core'; +import { render, waitFor } from '@testing-library/angular'; + +@Component({ + template: ` `, +}) +class LoopComponent { + get classes() { + return { + someClass: true, + }; + } +} + +test('does not end up in a loop', async () => { + await render(LoopComponent); + + await expect( + waitFor(() => { + expect(true).toEqual(false); + }), + ).rejects.toThrow(); +}); diff --git a/projects/testing-library/src/lib/testing-library.ts b/projects/testing-library/src/lib/testing-library.ts index 6c0a3812..e8447ba7 100644 --- a/projects/testing-library/src/lib/testing-library.ts +++ b/projects/testing-library/src/lib/testing-library.ts @@ -303,16 +303,23 @@ function addAutoImports({ imports, routes }: Pick, ' } /** - * Wrap waitFor to poke the Angular change detection cycle before invoking the callback + * Wrap waitFor to invoke the Angular change detection cycle before invoking the callback */ async function waitForWrapper( detectChanges: () => void, callback: () => T extends Promise ? never : T, options?: dtlWaitForOptions, ): Promise { + detectChanges(); return await dtlWaitFor(() => { - detectChanges(); - return callback(); + try { + return callback(); + } catch (error) { + if (error.name === 'TestingLibraryElementError') { + detectChanges(); + } + throw error; + } }, options); } diff --git a/projects/testing-library/tests/wait-for.spec.ts b/projects/testing-library/tests/wait-for.spec.ts index ce80b08d..116faaea 100644 --- a/projects/testing-library/tests/wait-for.spec.ts +++ b/projects/testing-library/tests/wait-for.spec.ts @@ -1,6 +1,6 @@ import { Component } from '@angular/core'; import { timer } from 'rxjs'; -import { render, screen, fireEvent, waitFor as waitForATL } from '../src/public_api'; +import { render, screen, fireEvent, waitFor } from '../src/public_api'; @Component({ selector: 'atl-fixture', @@ -24,7 +24,7 @@ test('waits for assertion to become true', async () => { fireEvent.click(screen.getByTestId('button')); - await waitForATL(() => screen.getByText('Success')); + await waitFor(() => screen.getByText('Success')); screen.getByText('Success'); }); @@ -33,7 +33,7 @@ test('allows to override options', async () => { fireEvent.click(screen.getByTestId('button')); - await expect(waitForATL(() => screen.getByText('Success'), { timeout: 200 })).rejects.toThrow( + await expect(waitFor(() => screen.getByText('Success'), { timeout: 200 })).rejects.toThrow( /Unable to find an element with the text: Success/i, ); }); From 8314aef895e0d4e0b3a4c20191571ebac2d58861 Mon Sep 17 00:00:00 2001 From: timdeschryver <28659384+timdeschryver@users.noreply.github.com> Date: Fri, 25 Jun 2021 11:04:11 +0200 Subject: [PATCH 2/2] fix: don't end up in an infinite loop --- projects/testing-library/src/lib/testing-library.ts | 8 ++++---- .../tests/issues/{188.spec.ts => issue-188.spec.ts} | 0 .../testing-library/tests}/issues/issue-230.spec.ts | 10 ++++++++-- .../tests/issues/{67.spec.ts => issue-67.spec.ts} | 0 4 files changed, 12 insertions(+), 6 deletions(-) rename projects/testing-library/tests/issues/{188.spec.ts => issue-188.spec.ts} (100%) rename {apps/example-app/src/app => projects/testing-library/tests}/issues/issue-230.spec.ts (56%) rename projects/testing-library/tests/issues/{67.spec.ts => issue-67.spec.ts} (100%) diff --git a/projects/testing-library/src/lib/testing-library.ts b/projects/testing-library/src/lib/testing-library.ts index e8447ba7..168bd0d7 100644 --- a/projects/testing-library/src/lib/testing-library.ts +++ b/projects/testing-library/src/lib/testing-library.ts @@ -315,9 +315,7 @@ async function waitForWrapper( try { return callback(); } catch (error) { - if (error.name === 'TestingLibraryElementError') { - detectChanges(); - } + setImmediate(() => detectChanges()); throw error; } }, options); @@ -347,8 +345,10 @@ async function waitForElementToBeRemovedWrapper( } return await dtlWaitForElementToBeRemoved(() => { + const result = cb(); detectChanges(); - return cb(); + setImmediate(() => detectChanges()); + return result; }, options); } diff --git a/projects/testing-library/tests/issues/188.spec.ts b/projects/testing-library/tests/issues/issue-188.spec.ts similarity index 100% rename from projects/testing-library/tests/issues/188.spec.ts rename to projects/testing-library/tests/issues/issue-188.spec.ts diff --git a/apps/example-app/src/app/issues/issue-230.spec.ts b/projects/testing-library/tests/issues/issue-230.spec.ts similarity index 56% rename from apps/example-app/src/app/issues/issue-230.spec.ts rename to projects/testing-library/tests/issues/issue-230.spec.ts index 3ac54147..c1e5cf35 100644 --- a/apps/example-app/src/app/issues/issue-230.spec.ts +++ b/projects/testing-library/tests/issues/issue-230.spec.ts @@ -1,5 +1,5 @@ import { Component } from '@angular/core'; -import { render, waitFor } from '@testing-library/angular'; +import { render, waitFor, screen } from '@testing-library/angular'; @Component({ template: ` `, @@ -12,7 +12,7 @@ class LoopComponent { } } -test('does not end up in a loop', async () => { +test('wait does not end up in a loop', async () => { await render(LoopComponent); await expect( @@ -21,3 +21,9 @@ test('does not end up in a loop', async () => { }), ).rejects.toThrow(); }); + +test('find does not end up in a loop', async () => { + await render(LoopComponent); + + await expect(screen.findByText('foo')).rejects.toThrow(); +}); diff --git a/projects/testing-library/tests/issues/67.spec.ts b/projects/testing-library/tests/issues/issue-67.spec.ts similarity index 100% rename from projects/testing-library/tests/issues/67.spec.ts rename to projects/testing-library/tests/issues/issue-67.spec.ts