chapel
chapel copied to clipboard
chapel-lang
[Design] Should `setuptools` be listed in the `third-party/chpl-venv` dependencies for `start_test`?
setuptools was added as a dependency for start_test in this commit for Arkouda testing.
On the one hand, it's good to have dependencies listed in as few places as possible, to reduce complexity and make it easier to analyze what the dependencies of a project are. On the other hand, if it's only useful for one test directory, arguably it should be limited to that test directory - this will make it easier to recognize if it becomes no longer needed.
@jabraham17 - I strongly suspect you have additional concerns I haven't listed here :)
My primary concern is that we adjusted the dependencies for start_test (i.e. a test runner) to make 1 "test" pass (arkouda).
This is for 2 reasons
- from a clean code perspective, this feels very wrong. Why does one test infect everything else? The test should just handle getting the right dependencies
- are we covering up an issue in Arkouda that should be fixed?
- from a clean code perspective, this feels very wrong. Why does one test infect everything else? The test should just handle getting the right dependencies
Yes, but also, if multiple tests end up requiring the same thing, it's a lot harder to notice if they're all just handling it themselves. Having a centralized place helps avoid things like accidentally having one test depend on another directory running before it to get the dependencies it needs, or paying the cost of downloading and installing something twice if they would otherwise end up truly isolated. It's a tradeoff, imo
Yes, but also, if multiple tests end up requiring the same thing, it's a lot harder to notice if they're all just handling it themselves. Having a centralized place helps avoid things like accidentally having one test depend on another directory running before it to get the dependencies it needs, or paying the cost of downloading and installing something twice if they would otherwise end up truly isolated.
I somewhat agree. Having multiple tests have to duplicate dependency logic is bad, but in my opinion thats a problem for the test author, not the test runner. For example, this is why for the python numpy tests we have test/library/packages/Python/correctness/arrays/PRETEST which installs numpy for all the tests in the directory. It does not matter if you do start_test test/library/packages/Python/correctness/arrays or start_test test/library/packages/Python/correctness/arrays/someChapelFile.chpl, you still get the right dependencies installed once. This did not require editing start_test to achieve.
Needing to specially modify start_test to make a test work is a failure of start_test
It's a tradeoff, imo
I disagree with this, I think we can have our cake and eat it too :). We have the mechanisms in start_test to handle dependencies (maybe having to write a custom PRECOMP or PRETEST is not the cleanest thing in the world, but it works well) without needing to specialize start_test
For example, this is why for the python numpy tests we have
test/library/packages/Python/correctness/arrays/PRETESTwhich installs numpy for all the tests in the directory. It does not matter if you dostart_test test/library/packages/Python/correctness/arraysorstart_test test/library/packages/Python/correctness/arrays/someChapelFile.chpl, you still get the right dependencies installed once. This did not require editingstart_testto achieve.
Sure, but doesn't that potentially imply that numpy should be listed as a dependency of our testing system?
No, numpy is a dependency of the tests themselves.
I am making a distinction between start_test the tool and $CHPL_HOME/test, the Chapel test suite
Related to the last comment: We distribute start_test in releases, but not $CHPL_HOME/test (outside of release/examples).
We still list dependencies that are only for development. And numpy is also useful for the original Python interop, it's just something we expect users to install themselves.