Skip to content

Make a test more reliable #3300

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

Merged
merged 5 commits into from
Nov 13, 2022
Merged

Make a test more reliable #3300

merged 5 commits into from
Nov 13, 2022

Conversation

pepeiborra
Copy link
Collaborator

@pepeiborra pepeiborra commented Oct 23, 2022

"iface-error-test-1" fails occasionally in CI. The problem is that progress reporting and diagnostics arrive in non-deterministic ordering: a test that waits for both is creating a race condition.

Ideally we would solve the non-determinism problem for all tests, but I'm not sure how to do that. In the meantime, we should review all tests in the ghcide test suite. When fixing a test, prefer diagnostics over progress reporting since the former are more reliable.

@pepeiborra
Copy link
Collaborator Author

This needs #3301 to work properly

Copy link
Collaborator

@fendor fendor left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@michaelpj
Copy link
Collaborator

Worth a comment to explain why you're doing what you're doing in this case? Or a note in the test file to refer to in future?

@pepeiborra
Copy link
Collaborator Author

I'm planning to add a comment and land this but it might take me a few weeks

The diagnostics can arrive either before or after progress completed, so there's a race condition here: if they arrive after, then the test script will get confused
Progress and diagnostics can arrive in any order: a test that waits for both is creating a race condition
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