-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
separate module context for Compiler.jl tests #56636
base: master
Are you sure you want to change the base?
Conversation
Also need an |
When these files are run from |
Okay but in the future, we will likely run these tests in parallel on different workers and each of these workers needs to run the Line 5 in 0ded536
But could be a later thing to add. |
Yes, it’s probably related to parallel execution, so it might be a good idea to revisit this discussion in a PR that enables parallel execution for |
65d4b1b
to
b4acc79
Compare
Makes `test Compiler` work properly (as in use the Compiler package, not Base.Compiler) and pass tests, but still needs to be made parallel in a follow-on.
Co-authored-by: Kristoffer Carlsson <[email protected]>
00bc3e8
to
8261482
Compare
xref: <#56632 (review)> This also allows us to execute each file standalone, which might be useful for debugging.
b4acc79
to
a150de9
Compare
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'm ok with the setup_compiler change. For the module
wrapping though, the Base test infrastructure does that internally, so I'm not sure it will be required once we switch to that (which we need to for parallel execution).
xref: #56632 (review)
This also allows us to execute each file standalone, which might be useful for debugging.