-
Notifications
You must be signed in to change notification settings - Fork 5
Adopt subprocess fixture for testing. #391
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
Conversation
# Conflicts: # source/fab/tools/compiler.py # tests/unit_tests/steps/test_link.py # tests/unit_tests/tools/test_compiler.py # tests/unit_tests/tools/test_compiler_wrapper.py # tests/unit_tests/tools/test_linker.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only got through parts of it, will try to continue either later today or tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8000Still more to come, sorry
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I am stopping here and pass it back to you (extra long weekend) - main reason: could you explain to me the advantage of using fs
? I have the feeling I am missing something :) So basically, it is a temporary directory, that is just automatically cleared at the end of a test. In my (PSyclone-heavy ;) ) experience, I often use the files in tmp to debug failing tests. So if files are instead created in tmp, I can't debug them to see e.g. what might be wrong. We also have a fixture for changing into tmpdir, which seems to be the equivalent of fs, except in case of failures you can analyse the files. I would also prefer slightly smaller PRs ;) I am aware that this is already a split of an even larger one, but I am sure I missed a fs
fixture a couple of times before wondering what that might be ;) I'll pass it back to you at least for clarification of fs
, and pick it up next week.
Oh, also bringing it up to recent |
…to better reflect usage.
The thing which As a secondary advantage, being a whole filing system, you can use absolute filenames in safety which means you avoid having to add I take your point about debugging and access to the generated files but I think the run-time advantages out way them since tests are run much more often than they are written. |
Note that GitHub C.I. has become a little confused as the Python versions to test with has changed. This leaves old deprecated versions in the list of required runs even though they aren't. I can force commit once you give an approval. |
@MatthewHambley I've removed 3.7 and 3.8 builds from the branch protection rules for develop. Time to include 3.11, 3.12 (or even 3.13.3 if needed) in the GitHub Action workflow #405 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my comments. Nice update. Proceeding to merge.
Note that I have added this PR to the list of PRs to go into main (see #353). I think it would be best if we merge this in after the other 6 PRs that are currently waiting for review. It seems not to clash with the profiler profiles (surprisingly :) ). I think it's best if you don't delete the branch (and just updated it from main as the other 6 PRs go in), that should hopefully minimise conflicts. |
Rather than variations of the same hand-mocking of shelling out to a tool this change takes advantage of an existing fixture 676A . It also pulls in a fixture to create a fake filing system in memory which should be faster and less prone to faults based on what the developer has in their file tree.
Rather than variations of the same hand-mocking of shelling out to a tool this change takes advantage of an existing fixture.
It also pulls in a fixture to create a fake filing system in memory which should be faster and less prone to faults based on what the developer has in their file tree.