Skip to content

fix(tests): use os.utime for deterministic mtime in phase1 completion tests#41

Merged
pruiz merged 1 commit into
masterfrom
fix/test-mtime-race-wsl
Jun 4, 2026
Merged

fix(tests): use os.utime for deterministic mtime in phase1 completion tests#41
pruiz merged 1 commit into
masterfrom
fix/test-mtime-race-wsl

Conversation

@pruiz
Copy link
Copy Markdown
Owner

@pruiz pruiz commented Jun 4, 2026

Problem

test_phase1c_accepts_fresh_sandbox_state_with_existing_notes fails on WSL because time.time() has microsecond precision while filesystem st_mtime may be truncated to coarser granularity. Files created after run_start = time.time() can have st_mtime < run_start, causing _path_is_fresh() to return False.

The sister test test_phase1_check_patches_notes_root_and_sandbox_plan used a fragile time.time() - 2 workaround with the same root cause.

Fix

Replace timestamp arithmetic with explicit os.utime() calls that set deterministic mtimes relative to the captured start time:

  • Files expected to be staleos.utime(..., (run_start - 60, run_start - 60))
  • Files expected to be freshos.utime(..., (run_start + 60, run_start + 60))

This eliminates all dependence on filesystem clock granularity, platform differences, and test execution speed.

Testing

$ make tests
...609 passed in 66.46s...

Summary by CodeRabbit

  • Tests
    • Updated test setup for phase-completion scenarios with explicit timestamp management for sandbox-generated artifacts, plan files, and run summary files across multiple test cases.

… tests

Replace fragile 'time.time() - 2' hack with explicit os.utime() calls that set file mtimes deterministically relative to the captured start time. Artifacts expected to be stale get mtime = start - 60; fresh files get mtime = start + 60.

This eliminates mtime granularity races on WSL and other virtualized filesystems where st_mtime may have coarser resolution than time.time().
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a0f08ad0-db84-47d6-9e97-9c6907e2aab6

📥 Commits

Reviewing files that changed from the base of the PR and between 10a9b54 and 6dc0eb3.

📒 Files selected for processing (1)
  • tests/test_phases_completion.py

📝 Walkthrough

Walkthrough

This PR enhances two test functions in tests/test_phases_completion.py by adding explicit file timestamp control via os.utime() calls. The Phase 1/1c checklist test and Phase 1c fresh sandbox state test now set precise modification times on generated artifacts and run summaries to enable deterministic testing of phase completion logic.

Changes

Phase Completion Test Timestamp Control

Layer / File(s) Summary
Phase 1/1c checklist patching test - timestamp control
tests/test_phases_completion.py
Adds import os and introduces fake_time = time.time() with os.utime() calls to set controlled modification times on the sandbox markdown, plan file, phase artifacts, and run summary files.
Phase 1c fresh sandbox state test - timestamp control
tests/test_phases_completion.py
Adds import os and introduces run_start = time.time(), applying run_start - 60 to artifact initialization and run_start + 60 to generated markdown and Phase 1c run summary within the test block.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A timestamp here, a timestamp there,
Making test time crystal-clear!
Files dance through moments, controlled and neat,
Phase tests now march to time's own beat. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: fixing tests by using os.utime for deterministic modification times in phase completion tests.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/test-mtime-race-wsl

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 4, 2026

Coverage Report

Metric Value
Line Coverage 73.0%
Lines Covered 0 / 0

Download detailed HTML coverage reports per OS/Python from the workflow artifacts.

Generated by pytest-cov on 2026-06-04T19:40:14.566Z

@pruiz pruiz merged commit 963a2d9 into master Jun 4, 2026
7 checks passed
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.

1 participant