Skip to content

Fix failing test under Windows - #529

Open
dspinellis wants to merge 1 commit into
uutils:mainfrom
dspinellis:fix-Windows-test
Open

Fix failing test under Windows#529
dspinellis wants to merge 1 commit into
uutils:mainfrom
dspinellis:fix-Windows-test

Conversation

@dspinellis

Copy link
Copy Markdown
Collaborator

Replace NamedTempFile with a regular file inside a TempDir, so the file is closed and can be renamed or replaced by in-place editing on Windows. TempDir also simplifies cleanup by automatically removing the file, backup, and directory when dropped.

@sylvestre : Let's wait for all the CI results before merging this. The tests used to work fine. In the F command CI one of the tests when run for the coverage analysis failed, so this attempts to fix them with more robust handling of temporary files.

Replace `NamedTempFile` with a regular file inside a `TempDir`, so the
file is closed and can be renamed or replaced by in-place editing on
Windows. `TempDir` also simplifies cleanup by automatically removing
the file, backup, and directory when dropped.
Copilot AI lite review requested due to automatic review settings August 19, 2026 10:53
@sylvestre

Copy link
Copy Markdown
Contributor

sure, thanks

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

This PR updates sed in-place editing tests to avoid Windows file-handle/rename issues by replacing NamedTempFile usage with a closed regular file inside a TempDir.

Changes:

  • Replace NamedTempFile with tempfile::tempdir() + std::fs::write() to ensure files are closed before in-place editing.
  • Rely on TempDir drop for cleanup instead of manually closing/removing temp and backup files.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/by-util/test_sed.rs
Comment thread tests/by-util/test_sed.rs

// Get the file path before converting to TempPath
let path = temp.path().to_path_buf();
std::fs::write(&path, "hello, world\n")?;
Comment thread tests/by-util/test_sed.rs

let path = temp.path().to_path_buf();
let temp_path = temp.into_temp_path();
std::fs::write(&path, b"hello, world\n")?;
@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.18%. Comparing base (3f72f93) to head (baa3199).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #529   +/-   ##
=======================================
  Coverage   83.18%   83.18%           
=======================================
  Files          13       13           
  Lines        7001     7001           
  Branches      398      398           
=======================================
  Hits         5824     5824           
  Misses       1174     1174           
  Partials        3        3           
Flag Coverage Δ
macos_latest 83.83% <ø> (ø)
ubuntu_latest 84.03% <ø> (ø)
windows_latest 0.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

3 participants