Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure tests do not leave stale files in current directory. #2333

Merged
merged 1 commit into from
Oct 12, 2023

Conversation

digit-google
Copy link
Contributor

This patch fixes a minor but annoying issue during development where some tests would leave stale files in the current directory.

  • Introduce new ScopedFilePath class to perform remove-on-scope-exit of a given file path.

Fixes #1583

@digit-google
Copy link
Contributor Author

I am puzzled as to why ::_unlink() would fail to compile with MSVC (the compiler seems to think unlink was invoked). So I am going to rely on the unlink macro in util.h.

@digit-google digit-google force-pushed the remove-stale-test-outputs branch from 423e682 to a20170d Compare October 5, 2023 19:09
@digit-google
Copy link
Contributor Author

Ok, @jhasse , I have no idea what's going in with the failure to compile "unlink" / "_unlink" / "::_unlink" here with MSVC2017, any suggestion welcome, I am puzzled!

@jhasse
Copy link
Collaborator

jhasse commented Oct 6, 2023

No idea. My suggestion: #2338

I would use util.h though.

This patch fixes a minor but annoying issue during development
where some tests would leave stale files in the current
directory.

+ Introduce new ScopedFilePath class to perform
  remove-on-scope-exit of a given file path.

Fixes ninja-build#1583
@digit-google digit-google force-pushed the remove-stale-test-outputs branch from a20170d to eff5b44 Compare October 6, 2023 18:08
@digit-google
Copy link
Contributor Author

I think I was using util.h in a previous version that was uploaded here, and still failed.
Anyway I am looking impatiently for Appveyor and its zany configuration to be removed then :-)

@jhasse jhasse merged commit 7a8c494 into ninja-build:master Oct 12, 2023
9 checks passed
@jhasse jhasse added this to the 1.12.0 milestone Oct 12, 2023
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.

BuildTest.WrongOutputInDepfileCausesRebuild leaves build_log and ninja_deps behind
3 participants