-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Fix flaky unit test - remove 'Markup' object in data #5313
Conversation
@mistercrunch thanks for looking into this! for the frontend modifications, are existing slices guaranteed to not have markup? |
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.
LGTM
I like this code change, simplify |
I agree, good change but I think the real problem is that tests modify the db, and tests all share the same db. cc @john-bodley |
Went with @graceguo-supercat 's solution as far as fixing the tests go, but I feel like this cleans the regexes and we should get it through. |
Codecov Report
@@ Coverage Diff @@
## master #5313 +/- ##
==========================================
+ Coverage 61.32% 61.33% +<.01%
==========================================
Files 369 369
Lines 23488 23483 -5
Branches 2717 2713 -4
==========================================
- Hits 14405 14404 -1
+ Misses 9071 9067 -4
Partials 12 12
Continue to review full report at Codecov.
|
example of the failed test
https://travis-ci.org/apache/incubator-superset/jobs/397891630
@williaster