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

use dialog fixtures and fix flaky tests #1249

Merged
merged 1 commit into from Apr 27, 2021
Merged

use dialog fixtures and fix flaky tests #1249

merged 1 commit into from Apr 27, 2021

Conversation

sssoleileraaa
Copy link
Contributor

@sssoleileraaa sssoleileraaa commented Apr 23, 2021

Description

Fixes #1139

Test Plan

Confirm #1139 is fixed.

  • Check CI: https://app.circleci.com/pipelines/github/freedomofpress/securedrop-client?branch=1139-fix
    • confirm there are 10+ successful runs in a row for this PR branch (when CI used to fail ~50% of the time)
  • run script from STR section of Investigate flaky tests #1139 (takes approx 17 minutes if there are no failures: 90-120 seconds each test run)
    • confirm no dialog errors
  • modify script to execute make test instead of make test-random and modify how many times you loop if you're not satified with 10 (I've tested with 100)
  • run script (takes approx 7 minutes for 10 loops if there are no failures: 30-50 seconds each test run)
    • confirm no dialog errors

Makefile Outdated Show resolved Hide resolved
@sssoleileraaa
Copy link
Contributor Author

while super focused on flaky test errors I forgot about linter errors. ok, now this is ready for review.

@sssoleileraaa sssoleileraaa added this to Ready for Review in SecureDrop Team Board Apr 23, 2021
Signed-off-by: Allie Crevier <allie@freedom.press>
@emkll emkll moved this from Ready for Review to Under Review in SecureDrop Team Board Apr 26, 2021
@emkll
Copy link
Contributor

emkll commented Apr 26, 2021

Thanks @creviera for opening this, the tests do appear to be more reliable. With make test, tests are reliably passing. However, getting intermittent failures with make test-random:

________________________________________________________________________________________________________ test_SourceList_set_snippet _________________________________________________________________________________________________________
CALL ERROR: Exceptions caught in Qt event loop:
________________________________________________________________________________
Traceback (most recent call last):
  File "/home/user/src/securedrop-client/securedrop_client/gui/widgets.py", line 2545, in animate_header
    self.header_spinner_label.setPixmap(self.header_animation.currentPixmap())
RuntimeError: wrapped C/C++ object of type QLabel has been deleted
________________________________________________________________________________

@sssoleileraaa
Copy link
Contributor Author

@emkll curious how many times you had to run make test-random so I can try repro'ing

@sssoleileraaa
Copy link
Contributor Author

also, could you post your results of the make test runs (i ran 100 times without failure). make test-random I ran 10 times I think and I'm going to increase it now to 20 or more since it takes a long time.

Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you post your results of the make test

make test is working fine, I've tried over 20 runs and it's reliable

In sd-app, make test-random is now almost working as expected (had 1 failure due to deleted object in 20 runs)

I'm inclined to merge as-is based on my results described above and the variance in my local environment, as this is already a significant improvement

@emkll
Copy link
Contributor

emkll commented Apr 27, 2021

Merging per the conversation in standup today, thanks @creviera !

@emkll emkll merged commit 304d269 into main Apr 27, 2021
SecureDrop Team Board automation moved this from Under Review to Done Apr 27, 2021
@emkll emkll deleted the 1139-fix branch April 27, 2021 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Investigate flaky tests
2 participants