-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
address flaky Cypress test for text tiles #1488
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 We test this behavior in other tests so I don't think there's much value in leaving it commented out, so I'd recommend removing it entirely rather than leaving it commented out.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1488 +/- ##
==========================================
+ Coverage 85.23% 85.33% +0.10%
==========================================
Files 555 555
Lines 27601 27601
Branches 7558 7558
==========================================
+ Hits 23525 23554 +29
+ Misses 3920 3746 -174
- Partials 156 301 +145
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
codap-v3 Run #4344
Run Properties:
|
Project |
codap-v3
|
Branch Review |
flaky-text-tile-test
|
Run status |
Passed #4344
|
Run duration | 08m 28s |
Commit |
6ba8cc6787: deleted update text tile Cypress test
|
Committer | nstclair-cc |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
29
|
Skipped |
0
|
Passing |
213
|
View all changes introduced in this branch ↗︎ |
Hey @kswenson - I think the solution for the flaky test for now is to comment out the offending code. I manual tested and things look okay for now, but let me know if more steps are needed or if you see something more obvious. Feel free to merge if you agree.