[Test Only] Add a sleep to 'testInputModifiedDuringSingleJobBuild' to accomodate filesystems with insufficient timestamp precision #1810
Conversation
|
@swift-ci test |
| @@ -356,6 +356,10 @@ final class JobExecutorTests: XCTestCase { | |||
| XCTAssertEqual(jobs.count, 1) | |||
| XCTAssertTrue(jobs[0].requiresInPlaceExecution) | |||
|
|
|||
| // Sleep for 1s to allow for quiescing mtimes on filesystems with | |||
| // insufficient timestamp precision. | |||
| Thread.sleep(forTimeInterval: 1) | |||
There was a problem hiding this comment.
Issue (possibly-blocking): I cringe when I see "sleep" in a test as there will always be room for flakiness here.
Instead of relying of sleep, is there a more deterministic wait of determining when we are ready to proceed?
With sleep:
- we may not wait long enough, causing for intermittent failure
- the condition for waiting is met "quickly", causing the test to wait for no reason
I understand this is a quick solution, but it may be the cause of a flaky test, which is not ideal to have.
There was a problem hiding this comment.
Flakiness in this test after this change would mean a serious issue and any test failure would require investigation.
The test is designed to detect a change to a file being done in a specific interval of time, and to do so we need to ensure there actually is an interval of time. On some platforms, code execution between file creation and modification is a sufficient interval of time per filesystem timestamp granularity. On others, it is not so we must establish a time interval, as per other incremental compilation tests in this suite as well. It is true that the condition is met more quickly than the sleep interval, however it is also not a threshold that would meaningfully affect the parallel test suite runtime. The threshold of sleep here is picked very conservatively to ensure that "we may not wait long enough, causing for intermittent failure" does not happen. Meaning that "there will always be room for flakiness here" and "it may be the cause of a flaky test" is not true.
|
@swift-ci test Linux platform |
|
I think the fix is fine for using sleep, can we also update the check so it doesn't call |
… accomodate filesystems with insufficient timestamp precision
artemcm
force-pushed
the
NeedSleepToGetItRight
branch
from
ddd8d84 to
2169ff5
Compare
|
@swift-ci test |
|
@swift-ci test Windows platform |
Great idea. Done. |
|
@swift-ci test Windows platform |
No description provided.