fix: Fix flaky global breadcrumbs test #2676
Conversation
| return delay(); | ||
| await waitFor(() => expect(wrapper.findAppLayout()!.find('[data-awsui-discovered-breadcrumbs="true"]')).toBeTruthy()); |
There was a problem hiding this comment.
Main part of the fix. Instead of waiting for an arbitrary time, we wait for a clue from the component that the deduplication happened
| render(<BreadcrumbGroup items={defaultBreadcrumbs} />); | ||
| await delay(); |
There was a problem hiding this comment.
For the cases where we test no-deduplication mode we keep the delay, because there is no that clue
There was a problem hiding this comment.
The test "when multiple nested app layouts rendered, the outer instance receives breadcrumbs" is still flaky.
There was a problem hiding this comment.
And also this one: "when multiple breadcrumbs instances are present the latest is applied"
There was a problem hiding this comment.
And this: "renders breadcrumbs adjacent to app layout"
There was a problem hiding this comment.
Surprisingly, the tests passed in the experiment PR but they consistently failing locally. I have 10-15 failing tests for a single run.
There was a problem hiding this comment.
Applied the same describe.each multiplier, ran tests a few thousand times locally as well, all passes for me.
Do you consider your local issues merge blocking?
There was a problem hiding this comment.
Alright, it is not a blocker for the pr then. I use the same branch and run npm install - the tests do fail consistently for me still.
There was a problem hiding this comment.
Did you try this version, right?
I noticed you removed lines like expect(findAllBreadcrumbsInstances()).toHaveLength(1);. All failures I receive locally are due to these.
There was a problem hiding this comment.
The latest version with expect(findAllBreadcrumbsInstances()).toHaveLength(1) under waitFor does work for me locally, too.
just-boris
force-pushed
the
fix-global-breadcrumb-tests
branch
from
47a1ceb to
ad395de
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2676 +/- ##
==========================================
- Coverage 95.79% 95.78% -0.01%
==========================================
Files 744 744
Lines 20566 20566
Branches 7011 6639 -372
==========================================
- Hits 19702 19700 -2
- Misses 856 858 +2
Partials 8 8 ☔ View full report in Codecov by Sentry. |
just-boris
force-pushed
the
fix-global-breadcrumb-tests
branch
from
ad395de to
dd0aec6
Compare
Description
Replacement for this PR: #2666
Related links, issue #, if available: n/a
How has this been tested?
Build passes
Review checklist
The following items are to be evaluated by the author(s) and the reviewer(s).
Correctness
CONTRIBUTING.md.CONTRIBUTING.md.Security
checkSafeUrlfunction.Testing
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.