switch to @brightspace-ui/testing fixture #3764

dlockhart · 2023-06-20T21:36:01Z

The vast majority of the changes here are just adjusting the import location, however having fixture now wait for everything to be ready did require adjusting a few tests that were dependent on it not doing that.

Once this is merged I'll do a separate pull request to remove all the test code that's manually waiting for nested element updateCompletes.

github-actions · 2023-06-20T21:37:46Z

Thanks for the PR! 🎉

We've deployed an automatic preview for this PR - you can see your changes here:

URL	https://pr-3764-brightspace-ui-core.d2l.dev/

Note
The build needs to finish before your changes are deployed.
Changes to the PR will automatically update the instance.

dlockhart · 2023-06-20T21:37:01Z

components/dialog/test/dialog.test.js

@@ -17,7 +17,8 @@ describe('d2l-dialog', () => {
 	describe('focus management', () => {

 		it('should focus on close button if no focusable elements inside', async() => {
-			const el = await fixture(html`<d2l-dialog opened>not focusable</d2l-dialog>`);
+			const el = await fixture(html`<d2l-dialog>not focusable</d2l-dialog>`);
+			el.opened = true;


As discussed previously, these types of tests where an event is fired as part of boot-up need to be adjusted to control when the event gets fired themselves.

dlockhart · 2023-06-20T21:37:22Z

components/dropdown/test/dropdown-content.test.js

@@ -62,7 +61,6 @@ describe('d2l-dropdown', () => {
 	beforeEach(async() => {
 		dropdown = await fixture(normalFixture);
 		content = dropdown.querySelector('d2l-dropdown-content');
-		await content.updateComplete;


Will do more of this kind of thing in a follow-up PR.

dlockhart · 2023-06-20T21:38:14Z

components/hierarchical-view/test/hierarchical-view.test.js

@@ -24,7 +24,6 @@ describe('d2l-hierarchical-view', () => {
 		const el = await fixture(viewsFixture);
 		view1 = el.querySelector('#view1');
 		view2 = el.querySelector('#view2');
-		await oneEvent(view1, 'd2l-hierarchical-view-resize');


This event has already been fired by the time fixture resolves.

dlockhart · 2023-06-20T21:39:04Z

components/inputs/test/input-search.test.js

@@ -103,7 +103,7 @@ describe('d2l-input-search', () => {

 			let clock;
 			beforeEach(() => {
-				clock = useFakeTimers();
+				clock = useFakeTimers({ toFake: ['clearTimeout', 'setTimeout'] });


The new fixture relies on await nextFrame() when it resets stuff, so we need to be more targeted with our mocking otherwise requestAnimationFrame gets mocked.

This would be good to note on the eventual README docs

Yep I've already included it in the draft upgrade guide, but I could definitely see it going into a README as well.

dlockhart · 2023-06-20T21:40:01Z

components/list/test/list-item-drag-handle.test.js

These tests were VERY flaky in Firefox. I spent the entire day trying to figure out why occasionally the handle loses focus before the keys can be sent to it but got nowhere. So I ended up implementing a "retry".

We'll have to keep an eye on these, to see if one retry tends to be enough. Longer tests make flake that much more painful

dlockhart · 2023-06-20T21:40:33Z

components/list/test/list-item-drag-handle.test.js

-				actionArea.dispatchEvent(new Event('click'));
-			});
-			await oneEvent(actionArea, 'click');
+			element.activateKeyboardMode();


Activating keyboard mode does everything that clicking the button does.

dlockhart · 2023-06-20T21:41:45Z

components/menu/test/menu.test.js

 			}
 		);

-		const elem = await fixture(`
+		const elemPromise = fixture(`
 			<d2l-menu>
 				<d2l-menu-item text="fast"></d2l-menu-item>
 				<${delayedUpdateMenuItem}></${delayedUpdateMenuItem}>
 				<span>not a menu item</span>
 			</d2l-menu>
 		`);


Had to rework how this "slow element" works since fixture now won't resolve until everything is ready anyway.

dlockhart · 2023-06-20T21:42:11Z

components/more-less/more-less.js

-				bubbles: false,
-				composed: false
-			}))
-		);


No longer needed now that fixture waits for this for us.

dlockhart · 2023-06-20T21:42:54Z

mixins/labelled/test/label-mixin.test.js

@@ -92,7 +91,7 @@ describe('LabelledMixin', () => {
 			const labelledElem = elem.querySelector('[labelled-by="label1"]');
 			const labelElem = elem.querySelector('#label1');
 			labelElem.textContent = 'new label value';
-			await nextFrame();


This was really flaky so I switched it to use an event instead of rely on rAF.

dlockhart · 2023-06-21T14:10:36Z

.github/workflows/ci.yml

@@ -21,7 +21,7 @@ jobs:
        run: npm run lint:style
  playwright:
    name: Accessibility and Unit Tests
-    timeout-minutes: 10
+    timeout-minutes: 15


I've re-run this many times locally and in CI.

Locally, these changes are 1.1x (aXe) to 1.5x (unit) slower, which is barely noticeable

In CI however, these changes are 1.3x (aXe) to 1.8x (unit) slower, which is adding ~3.5 minutes to this job. It's still quicker than the visual-diff tests though! 😅

It's admittedly not great, but also not shocking since we're now await-ing all nested Lit components for ~1650 tests, and doing a lot more resetting of the browser state between tests. If the slowness becomes a problem, there's a lot of tests using timeouts that we could look into fixing up.

Yeah I think it's worth doing some cleanup to get a better sense of the real timing

svanherk · 2023-06-21T17:37:31Z

.github/workflows/ci.yml

@@ -21,7 +21,7 @@ jobs:
        run: npm run lint:style
  playwright:
    name: Accessibility and Unit Tests
-    timeout-minutes: 10
+    timeout-minutes: 15


Yeah I think it's worth doing some cleanup to get a better sense of the real timing

svanherk · 2023-06-21T17:40:58Z

components/inputs/test/input-search.test.js

@@ -103,7 +103,7 @@ describe('d2l-input-search', () => {

 			let clock;
 			beforeEach(() => {
-				clock = useFakeTimers();
+				clock = useFakeTimers({ toFake: ['clearTimeout', 'setTimeout'] });


This would be good to note on the eventual README docs

components/list/test/list-item-drag-handle.test.js

svanherk · 2023-06-21T18:02:13Z

components/list/test/list-item-drag-handle.test.js

We'll have to keep an eye on these, to see if one retry tends to be enough. Longer tests make flake that much more painful

Co-authored-by: Stacey Van Herk <[email protected]>

ghost · 2023-06-21T18:45:18Z

🎉 This PR is included in version 2.131.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

dlockhart commented Jun 20, 2023

View reviewed changes

switch to @brightspace-ui/testing fixture

dab2506

dlockhart force-pushed the dlockhart/use-brightspace-ui-testing branch from b2b9802 to dab2506 Compare June 20, 2023 21:45

github-actions bot mentioned this pull request Jun 20, 2023

Updating Visual Diff Goldens for PR 3764 #3765

Closed

increase timeout

01f21ab

dlockhart commented Jun 21, 2023

View reviewed changes

dlockhart marked this pull request as ready for review June 21, 2023 14:10

dlockhart requested a review from a team as a code owner June 21, 2023 14:10

BrightspaceUI deleted a comment from github-actions bot Jun 21, 2023

svanherk approved these changes Jun 21, 2023

View reviewed changes

Update components/list/test/list-item-drag-handle.test.js

cebf96a

Co-authored-by: Stacey Van Herk <[email protected]>

dlockhart enabled auto-merge (squash) June 21, 2023 18:30

dlockhart merged commit ffdfcb8 into main Jun 21, 2023

dlockhart deleted the dlockhart/use-brightspace-ui-testing branch June 21, 2023 18:32

ghost added the released label Jun 21, 2023

bearfriend pushed a commit that referenced this pull request Jun 27, 2023

switch to @brightspace-ui/testing fixture (#3764)

2325ffb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

switch to @brightspace-ui/testing fixture #3764

switch to @brightspace-ui/testing fixture #3764

dlockhart commented Jun 20, 2023

github-actions bot commented Jun 20, 2023

dlockhart Jun 20, 2023

dlockhart Jun 20, 2023

dlockhart Jun 20, 2023

dlockhart Jun 20, 2023

svanherk Jun 21, 2023

dlockhart Jun 21, 2023

dlockhart Jun 20, 2023

svanherk Jun 21, 2023

dlockhart Jun 20, 2023

dlockhart Jun 20, 2023

dlockhart Jun 20, 2023

dlockhart Jun 20, 2023

dlockhart Jun 21, 2023

svanherk Jun 21, 2023

svanherk Jun 21, 2023

svanherk Jun 21, 2023

svanherk Jun 21, 2023

ghost commented Jun 21, 2023

switch to @brightspace-ui/testing fixture #3764

switch to @brightspace-ui/testing fixture #3764

Conversation

dlockhart commented Jun 20, 2023

github-actions bot commented Jun 20, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Jun 21, 2023