Fix tests on Windows #5096
Fix tests on Windows #5096
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| waitForHello(bboxWithMinimumDuration); | ||
|
|
||
| assertThat(bboxWithMinimumDuration.isRunning()).isTrue(); | ||
|
|
||
| waitForHello(bboxWithMinimumDuration); |
There was a problem hiding this comment.
I think it was wrong previously: once waitForHello returns, sleep && echo would complete and the container should stop. So it is wrong to expect isRunning() to return true here.
It worked fine before on Linux probably because the check was faster than containers' cleanup but I got a flaky behaviour on Windows
There was a problem hiding this comment.
I think you are right. Do you remember your original idea here @rnorth?
There was a problem hiding this comment.
I wasn't the original author, but it looks to me as though the original version was indeed prone to a race condition. It's quite surprising that it's taken so long for this to be detected.
kiview
left a comment
kiview
left a comment
There was a problem hiding this comment.
Very happy to further polish our Windows support (even if in this case, this is not about actual implementation code, but just making our own tests more portable).
| waitForHello(bboxWithMinimumDuration); | ||
|
|
||
| assertThat(bboxWithMinimumDuration.isRunning()).isTrue(); | ||
|
|
||
| waitForHello(bboxWithMinimumDuration); |
There was a problem hiding this comment.
I think you are right. Do you remember your original idea here @rnorth?
No description provided.