Skip to content

Conversation

@HofmeisterAn
Copy link
Contributor

What does this PR do?

Synchronizes the Docker system event tests with the help of AutoResetEvent. At least two tests (MonitorEventsAsync_IsCancelled_NoStreamCorruption and MonitorEventsFiltered_Succeeds) do not synchronize the events. The tests are not deterministic, they might run into a race condition where events are not published on assertions yet.

Why is it important?

Some Docker.DotNet tests are flaky, PR improves the reliability of the tests.

Related issues

Follow-ups

  • Looking closer into the tests, I noticed (gut feeling) events triggered, quickly after another might not be published reliable. It looks like they are swallowed.
  • I think we can reduce the complexity of StreamUtil.
  • /cc @dgvives You might want to take a look at the stream corruption test changes.

@dgvives
Copy link
Contributor

dgvives commented Dec 28, 2022

@HofmeisterAn the stream corruption tests come from #375, I didn't run into this problem myself but I was able to replicate it using that piece of code.
I would say tests are good if no stream corruption happens

@HofmeisterAn
Copy link
Contributor Author

@HofmeisterAn the stream corruption tests come from #375, I didn't run into this problem myself but I was able to replicate it using that piece of code. I would say tests are good if no stream corruption happens

I noticed that the events are not published reliable and fixed them too. If you e.g. look at the other PR's build (ff.) you see no event or message output. I got them sometimes on my local runs though.

@HofmeisterAn
Copy link
Contributor Author

@galvesribeiro can you please take a look into this PR? I would like to work on some follow-ups.

@HofmeisterAn
Copy link
Contributor Author

@galvesribeiro can you please review the PR?

@HofmeisterAn HofmeisterAn deleted the feature/fix-flakey-tests branch October 30, 2024 05:11
@HofmeisterAn HofmeisterAn restored the feature/fix-flakey-tests branch October 30, 2024 05:11
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.

2 participants