Avoid duplicate includes causing warnings in tests
#689
Conversation
Currently we're calling `include` in a loop over supported backends. The files being `include`d contain definitions which get overwritten when `include` is called a second time, resulting in a lot of warnings that make the test logs noisy. The approach taken here to address this is to wrap things in expressions, define an anonymous module inside of the loop over backends, `Core.eval` the expressions into the anonymous module, and `Core.include` the files into it. There are better ways to do this but this *should* get the job done with minimal required changes.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
A bit cursed but does seem to work |
There was a problem hiding this comment.
I personally think this makes the testing quite a bit more messy/hard to follow. Having some comments explaining all the evals would certainly help, but I think I'd much prefer splitting things up into proper test-util submodules that can be loaded normally in teh top-level runtests.jl and then relative imported in other submodules. I know it's a bit more rearranging, but I think ti would at least be more approachable/sensible for others who come along and want to contribute.
|
100% agree with that. This was the path of least effort, haha. I'll see if I can revisit this soon but in the meantime, for anybody reading this, do feel free to implement Jacob's suggestion in your own PR. |
Currently we're calling
includein a loop over supported backends. The files beingincluded contain definitions which get overwritten whenincludeis called a second time, resulting in a lot of warnings that make the test logs noisy. The approach taken here to address this is to wrap things in expressions, define an anonymous module inside of the loop over backends,Core.evalthe expressions into the anonymous module, andBase.includethe files into it. There are better ways to do this but this should get the job done with minimal required changes.Fixes #685.