Skip to content

fix: restore CWD before TemporaryDirectory cleanup on Windows#281

Merged
danielmeppiel merged 1 commit intomicrosoft:mainfrom
sergio-sisternes-epam:fix/windows-ci-tempdir-cleanup
Mar 13, 2026
Merged

fix: restore CWD before TemporaryDirectory cleanup on Windows#281
danielmeppiel merged 1 commit intomicrosoft:mainfrom
sergio-sisternes-epam:fix/windows-ci-tempdir-cleanup

Conversation

@sergio-sisternes-epam
Copy link
Collaborator

Description

Fix Windows CI failure introduced by PR #227. All 7 tests in TestInstallCommandAutoBootstrap fail on windows-latest with PermissionError: [WinError 32] because TemporaryDirectory cleanup cannot delete a directory that is the process's current working directory.

Root cause: Each test calls os.chdir(tmp_dir) inside a with tempfile.TemporaryDirectory() block. When the context manager exits, it tries to rmtree the temp directory — but on Windows, this fails because the CWD still points there. On Unix this silently succeeds, so it was never caught before.

Fix: Wrap each test body in try/finally to restore CWD before TemporaryDirectory.__exit__ runs.

CI run: https://github.com/microsoft/apm/actions/runs/23044256406/job/66929873022

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

@sergio-sisternes-epam sergio-sisternes-epam marked this pull request as ready for review March 13, 2026 10:04
Copilot AI review requested due to automatic review settings March 13, 2026 10:04
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a Windows-only unit test failure where TemporaryDirectory cleanup fails if the process CWD is still inside the temp folder, by restoring the original working directory before the context manager exits.

Changes:

  • Wrap each TemporaryDirectory() test body in try/finally to restore CWD before temp cleanup runs (Windows-safe).
  • Ensure all tests in TestInstallCommandAutoBootstrap follow the same CWD restoration pattern.

You can also share your feedback on Copilot code review. Take the survey.

…ix emoji assertions

- Extract duplicated try/finally CWD-restore into _chdir_tmp() context manager
  (addresses Copilot review feedback)
- Fix integration test emoji assertions: replace emoji-dependent
  '✨ Package installed and ready to run' checks with the stable
  'Package installed and ready to run' substring after PR microsoft#227
  replaced emoji with ASCII in CLI output

Fixes Windows PermissionError [WinError 32] when TemporaryDirectory
cleanup runs while the process CWD is inside the temp dir.
@sergio-sisternes-epam sergio-sisternes-epam force-pushed the fix/windows-ci-tempdir-cleanup branch from a3bd59e to 13c9c8c Compare March 13, 2026 10:42
@danielmeppiel danielmeppiel merged commit a000d35 into microsoft:main Mar 13, 2026
6 of 7 checks passed
@sergio-sisternes-epam sergio-sisternes-epam deleted the fix/windows-ci-tempdir-cleanup branch March 13, 2026 11:01
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