This is an archive of the discontinued LLVM Phabricator instance.

[lit] Add test directory to sys.path before invoking it.
AcceptedPublic

Authored by zturner on Nov 16 2018, 10:17 AM.

Details

Summary

Test suites with complicated configuration logic may wish to split this logic up over multiple packages / modules. This is currently tricky because in order to do import x from the same directory, the directory has to be in sys.path. What I'd really like is to find a way to make the test suite an actual package. It seems that due to the way we compile / exec it, it's still part of the lit.llvm package. (For example, the value of __package__ from within one of these files is 'lit.llvm'. It would be nice if we could detach it from the top-level lit package and make it be its own package. This way from . import foo would "just work".

Anyway, that's for another day, for now this at least allows it to work.

Diff Detail

Event Timeline

zturner created this revision.Nov 16 2018, 10:17 AM
llvm/utils/lit/lit/TestingConfig.py
109

Won't this always reset the path back to the original path since the finally block always executes?

zturner added inline comments.Nov 16 2018, 10:41 AM
llvm/utils/lit/lit/TestingConfig.py
109

Yes, but I think that's actually what we want. In fact I think not doing this is a bug that's always been present. i.e. even if we don't append our own path, we should still be doing this. Imagine a case where the code you exec / compile modifies sys.path. Then later we run another test suite, it will have had its sys.path affected by the run of the first test suite. Since sys.path is a module level property, any test suite could affect any other test suite this way.

I don't think any test suites are currently modifying sys.path so this has never been a problem before, but now that we are explicitly modifying it, we definitely don't want test suite B to be affected by test suite A.

This revision is now accepted and ready to land.Nov 16 2018, 11:05 AM