Flaky UT Fix - #380
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts a unit test timing delay intended to reduce flakiness when asserting that a status file’s mtime changes after an update.
Changes:
- Increased
time.sleep()intest_update_filefrom0.03to0.08seconds to try to exceed filesystem mtime granularity.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| time.sleep(0.03) # ensure filesystem mtime granularity is exceeded | ||
| time.sleep(0.08) # ensure filesystem mtime granularity is exceeded | ||
| ext_status_handler.update_file(file_name) | ||
| stat_file_name = os.stat(os.path.join(dir_path, file_name + ".status")) | ||
| modified_time = stat_file_name.st_mtime |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #380 +/- ##
==========================================
- Coverage 94.87% 94.87% -0.01%
==========================================
Files 111 111
Lines 20855 20844 -11
==========================================
- Hits 19787 19776 -11
Misses 1068 1068
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Co-authored-by: yashnap <267671466+yashnap@users.noreply.github.com>
| prev_modified_time = stat_file_name.st_mtime | ||
| original_status_json = ext_status_handler.read_file(file_name) | ||
|
|
||
| time.sleep(0.02) |
There was a problem hiding this comment.
Please review removal of these lines with Rajasi Rane (@rane-rajasi)
There was a problem hiding this comment.
Just for context : I added the sleep time in this PR : https://github.com/Azure/LinuxPatchExtension/pull/376/changes. Earlier this test was disabled/skipped in github because of the assertion issue caused by time.
Koshy John (kjohn-msft)
left a comment
Koshy John (kjohn-msft)
left a comment
There was a problem hiding this comment.
Comment inline
DESCRIPTION BY COPILOT AGENT:
The issue I fixed was a flaky unit test (test_update_file) that depended on:
sleep() timing, and file mtime comparisons.
On some CI filesystems, timestamp granularity is coarse (e.g., 1 second), so the file modification time didn’t always change as expected, causing intermittent failures even when behavior was correct.
I changed the test to be deterministic by validating status file content instead:
Confirm update_file("test1") does not change test.status.
Confirm update_file(file_name) updates test.status content as expected.
Description update
Reworked test_update_file in Test_ExtOutputStatusHandler.py to remove timing/mtime-based assertions that caused CI flakiness due to filesystem timestamp granularity.
The test now validates behavior using status-file content comparisons, ensuring deterministic verification that:
updating a different sequence number does not affect the original status file, and
updating the target sequence number correctly updates status payload fields.