Docker.DotNet.Tests.ISystemOperationsTests.MonitorEventsAsync_Succeeds test fails
- Dominant language
- C#
- Stars
- 2.4k
- Forks
- 416
- PR merge metrics
- No merged PRs in 30d
Description
**Steps to reproduce the issue:**
1. Submit a PR
2. Sometimes the CI build fails
**What actually happened?:**
Test case fails despite no changes to relevant code. See https://github.com/dotnet/Docker.DotNet/pull/496/checks?check_run_id=1992119140 for an example.
**What did you expect to happen?:**
Test case passes.
**Additional information:**
I see that this is a known bug in the test case. "[// On CI/CD Pipeline exception is thrown, not always](https://github.com/dotnet/Docker.DotNet/blob/master/test/Docker.DotNet.Tests/ISystemOperations.Tests.cs#L212)"
I think there's more of a problem here than I'm willing to fix:
1. It _looks_ like the event streams are delayed. Some of the events from CreateImageAsync don't arrive until after MonitorEventsAsync starts, so they [show up in _onJSONMessageCalled](https://github.com/dotnet/Docker.DotNet/pull/496/checks#step:5:16). Some of the events from TagImageAsync don't arrive until [after the assertion](https://github.com/dotnet/Docker.DotNet/pull/496/checks#step:5:39).
2. I can't be _sure_ that that's what's happening, since stdio might be buffered too. Adding some Console.Out.Flush and Console.Error.Flush calls between commands can help check for that.
3. This _probably_ isn't what's going wrong in this case, but it's a bug anyway: Using MonitorEventsAsync this way has a race condition. Unless you `await` MonitorEventsAsync's Task, you can't guarantee that the monitoring has started, so you can miss messages. The fix would is to replace MonitorEventsAsync with something like StartEventMonitorAsync that returns a subscription.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.