test(e2e): Add tests for Recent Objects #6270
Conversation
- Test for 'target button' scroll and animation - Test for persistence on refresh - Test for displaying objects and aliases uniquely
Codecov Report
@@ Coverage Diff @@
## master #6270 +/- ##
=======================================
Coverage 55.14% 55.14%
=======================================
Files 607 607
Lines 26108 26108
Branches 2381 2381
=======================================
Hits 14398 14398
Misses 11040 11040
Partials 670 670
*This pull request uses carry forward flags. Click here to find out more. Continue to review full report at Codecov.
|
| @@ -50,14 +54,9 @@ test.describe('Recent Objects', () => { | |||
| await page.mouse.move(0, 100); | |||
There was a problem hiding this comment.
I can't leave a comment on line #48 but do we have a better locator for this element?
There was a problem hiding this comment.
updated to not use nth-child. A bit more deterministic
| // Creating 21 objects takes a while, so increase the timeout | ||
| test.slow(); | ||
|
|
||
| // Create 19 more objects (2 in beforeEach() + 19 new = 21 total) |
There was a problem hiding this comment.
If this is the case, i would add a
// Assert that two recent objects are already available
expect(await recentObjectsList.getByRole('listitem', { name: clock.name }).count()).toBe(2);
at the very beginning of the test
| @@ -54,6 +54,7 @@ | |||
| <div class="c-recentobjects-listitem__target-button"> | |||
| <button | |||
| class="c-icon-button icon-target" | |||
| :aria-label="`Open and scroll to ${domainObject.name}`" | |||
There was a problem hiding this comment.
back to our id conversation, is it possible to reference domainObject.name as an id and use aria-labelledby to compute this aria-label?
There was a problem hiding this comment.
domainObject.name is actually not unique, so wouldn't make a very good id, but I wonder if we can use the navigationPath instead as an id and reference that? we could then use aria-labelledby.
lemme play around with that
There was a problem hiding this comment.
So we can use something like recent-object:${navigationPath} to make a unique id for these list items, but we would still be missing a meaningful, unique a11y label to label it with. When you get some time let's chat about and revisit this
| @@ -24,7 +24,7 @@ | |||
| <ul | |||
| v-if="orderedPath.length" | |||
| class="c-location" | |||
| :aria-label="`${domainObject.name} Breadcrumb`" | |||
| :aria-label="`${domainObject.name}`" | |||
| } | ||
|
|
||
| // Assert that the list contains 20 objects | ||
| expect(await recentObjectsList.locator('.c-recentobjects-listitem').count()).toBe(20); |
There was a problem hiding this comment.
is there anything else worth asserting at this point given the test execution time?
There was a problem hiding this comment.
Updated the test to do another test for the "target" button, this time on an extremely deeply nested object
There was a problem hiding this comment.
This is really comprehensive. I just wish we had the codecov inline reporter to show us what was missed. :)
Just a few questions, and a suggestion for a better locator on line 48.
I think we might want to add a single visual test for a "deeply nested object" within the tree and recent objects components. Could you put in a stub?
- Do deep nesting of objects instead of flat objects - Collapse the tree completely and then test the "target" button for the most deeply nested item
Closes #6150
Describe your changes:
Adds tests for Recent Objects to cover the following:
All Submissions:
Author Checklist
Reviewer Checklist