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

Fix integration test race conditions #943

Merged

Conversation

engelmi
Copy link
Member

@engelmi engelmi commented Sep 16, 2024

This PR addresses two issues:

Fix proxy service propagate failure test

Relates to: #874

The proxy service propagates failure integration test has been flaky due to a race condition where either the job of the requesting service gets canceled or the requesting service maps the inactive state due to the BindsTo=. In order to account for this race condition, lets check if either case occurred.

Fix race condition in status watch test

In the integration test for the bluechictl status watch command two parallel processes are run - one producing output, the second checking it. To ensure that the processing part can collect all the output from process one, a short delay is added.

@coveralls
Copy link

coveralls commented Sep 16, 2024

Coverage Status

coverage: 83.569% (-0.1%) from 83.682%
when pulling 36f1ac8 on engelmi:fix-integration-test-race-conditions
into 1fc7741 on eclipse-bluechi:main.

@engelmi engelmi force-pushed the fix-integration-test-race-conditions branch from 3f2b800 to 24b4ce6 Compare September 16, 2024 10:38
@engelmi
Copy link
Member Author

engelmi commented Sep 16, 2024

/packit test

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

LGTM

@engelmi
Copy link
Member Author

engelmi commented Sep 16, 2024

@mwperina Thanks for the approval!
I'll trigger a few more runs on TF and in the GitHub CI to (kind of) see that the fix is working - locally I wasn't able to reproduce these race conditions. If ~5 consecutive runs are successful, I'll merge.

@engelmi
Copy link
Member Author

engelmi commented Sep 16, 2024

/packit test

@engelmi
Copy link
Member Author

engelmi commented Sep 16, 2024

@mwperina PTAL
I added another small commit to fix the wrong name of the integration test for user bus. Also, the fix for the status watch wasn't fully working... should now, hopefully.

Copy link
Member

@mwperina mwperina left a comment

Choose a reason for hiding this comment

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

LGTM

Relates to: eclipse-bluechi#874

The proxy service propagates failure integration test has been flaky due to
a race condition where either the job of the requesting service gets canceled
or the requesting service maps the inactive state due to the BindsTo=.
In order to account for this race condition, lets check if either case occurred.

Signed-off-by: Michael Engel <[email protected]>
@engelmi engelmi force-pushed the fix-integration-test-race-conditions branch from a11d29e to 36f1ac8 Compare September 16, 2024 14:45
@engelmi
Copy link
Member Author

engelmi commented Sep 16, 2024

I split out the bluechictl status watch: #946
@mkemel Maybe you could have a look at it?

@engelmi
Copy link
Member Author

engelmi commented Sep 16, 2024

/packit test

@engelmi
Copy link
Member Author

engelmi commented Sep 17, 2024

The packit Liberror issue is not solved yet. Since the changes only affect integration tests, the build isn't affected anyway. Also, the fix for the proxy propagate failure test seems to work.
Therefore, lets merge.

@engelmi engelmi merged commit 12cecdf into eclipse-bluechi:main Sep 17, 2024
19 of 21 checks passed
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