Check a different set of env vars, don't check the exact value
of the fallback path. (GetTempPath falls back to returning c:\windows
if nothing better is available in env vars.)
Details
- Reviewers
curdeius • Quuxplusone - Group Reviewers
Restricted Project - Commits
- rGb5e228fc00b1: [libcxx] [test] Fix the temp_directory_path test for windows
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
LGTM if you clarify the comment.
libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp | ||
---|---|---|
122 | Nit: capital in Windows. | |
122 | I'd rather go for a precise piece of information in the comment, as you wrote in the revision summary, the fallback is the Windows folder (https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettemppatha#remarks). |
libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp | ||
---|---|---|
122 | Nit: the Windows folder is not necessarily C:\Windows. Don't bother to change it before landing. |
libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp | ||
---|---|---|
122 | Oh, right, that's true, thanks! |
libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp | ||
---|---|---|
67 | Is there any significance to starting with dir2 on Windows versus dir1 on POSIX? Also, this is scope creep, but do you maybe want to (in a separate PR) make a test case to show that the setting of env var TMPDIR is correctly ignored on Windows? |
libcxx/test/std/input.output/filesystems/fs.op.funcs/fs.op.temp_dir_path/temp_directory_path.pass.cpp | ||
---|---|---|
67 | Not really, just copy/paste laziness, I can change it to start with dir1 there too. Hmm, I guess I could try to extend the test to check for ignored variables too, as a later patch. |
LGTM after changing to dir1 etc.; please double-check that the tests still pass after that change, then ship it!
Is there any significance to starting with dir2 on Windows versus dir1 on POSIX?
Also, this is scope creep, but do you maybe want to (in a separate PR) make a test case to show that the setting of env var TMPDIR is correctly ignored on Windows?