Skip to content
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

Merged
merged 1 commit into from
Jul 12, 2023

Conversation

kdmccormick
Copy link
Member

@kdmccormick kdmccormick commented Jul 11, 2023

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.

@kdmccormick
Copy link
Member Author

It passed the first time. I'll try a few more times.

@kdmccormick
Copy link
Member Author

It passed 5 times in a row.

@kdmccormick kdmccormick marked this pull request as ready for review July 12, 2023 02:15
@kdmccormick kdmccormick force-pushed the revert-32697-robrap/remove-flaky-cms-test branch from fc8cb45 to a0771dc Compare July 12, 2023 14:13
@kdmccormick
Copy link
Member Author

7 times in a row total, including once after rebasing this morning. No failures so far.

Copy link
Contributor

@bszabo bszabo left a 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.

@bszabo bszabo merged commit 86eb49a into master Jul 12, 2023
@bszabo bszabo deleted the revert-32697-robrap/remove-flaky-cms-test branch July 12, 2023 16:00
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

1 similar comment
@edx-pipeline-bot
Copy link
Contributor

2U Release Notice: This PR has been deployed to the edX production environment.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants