-
Notifications
You must be signed in to change notification settings - Fork 4k
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
revert: temp: remove flaky tests video/transcripts/test_views.py #32719
Conversation
It passed the first time. I'll try a few more times. |
It passed 5 times in a row. |
)" This reverts commit 6b19eab.
fc8cb45
to
a0771dc
Compare
7 times in a row total, including once after rebasing this morning. No failures so far. |
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.
Per slack discussion, we're going to reintroduce this test, and do a deeper dive if it starts failing again.
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
2U Release Notice: This PR has been deployed to the edX production environment. |
2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production. |
2U Release Notice: This PR has been deployed to the edX production environment. |
1 similar comment
2U Release Notice: This PR has been deployed to the edX production environment. |
This PR adds back a JS unit test which was identified as flaky in #32690 and removed by #32697.
We are not exactly sure why the test became flaky, although there is speculation that it is related to a Sass build refactoring. Since that refactoring merged, there have been two follow-up fixes, both related to the video block:
I posit that the test became flaky due to a styling bug which the Sass refactoring PR introduced, and was then fixed via the two follow-ups PR. We don't have an easy way to prove or disprove this hypothesis.
Since this PR is consistently passes JavaScript tests, though, I propose we merge it, and then immediately revert again & investigate more deeply if the test returns to being flaky.