Due to how the LIT deals with module files,
this change stores and runs the scripts in
their own temporary directory to prevent
interference in-between different tests.
It also makes `test_symbols.py` be more
consistent with the other scripts.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Males sense, thank you for working on this! I've made a couple of small suggestions.
| flang/test/Semantics/test_symbols.py | ||
|---|---|---|
| 5–11 | I'm suggesting a small edit to make it clearer what the input arguments to this script are. | |
| 22 | I'm suggesting a bit more descriptive variable here. tmp could mean a temporary file, directory, socket etc. | |
Is there a reason to use lit's suggestion for a temporary directory instead using tempfile.TemporaryDirectory or tempfile.mkdtemp?
| flang/test/Semantics/test_symbols.py | ||
|---|---|---|
| 44–45 | ||
I decided to use what's already in the LIT and it also more accurately reflects the behaviour of the bash script. Besides that, I don't see a reason why I couldn't use the tempfile library.
I like this suggestion - with tempfile.TemporaryDirectory the tests wouldn't require the extra argument (%t). Is the temporary directory name guaranteed to be unique to every test? That wasn't 100% obvious to me from the docs.
%t expands to <builddir>/tools/flang/test/<filename>.tmp. That is, it is unique for every test file, but that same for multiple RUN lines in the file and between make check invocations. However, I think it is better to always have a fresh temporary directory, flang-new does not need to see the leftover files from the previous make check run, those should be independent.
I'm suggesting a small edit to make it clearer what the input arguments to this script are.