https://pantsbuild.org/ logo
Join Slack
Powered by
Hm, I've been working on adding Shunit2 to the new...
# development
h
Hm, I've been working on adding Shunit2 to the new Bash plugin for tests, but the below is giving me pause and making me wonder if we should add shunit2 or not..
it matters which shell you run the tests with, and a common pattern is to run the same tests with multiple, e.g. 
bash
vs. 
zsh
. You control this by running your test file with a different shell, typically via shebang. You don't run 
shunit2 my_test.sh
, but rather add 
source ./shunit2
to 
my_test.sh
then run 
bash my_test.sh
or use the shebang -- The problem is how Pants should approach this. Iiuc, we aren't supposed to simply set PATH and let the shebang do its thing? @fast-nail-55400 is that correct? I don't think Pants trying to parse the shebang is a great idea A field on a 
shell_tests
target sounds a bit clunky if it duplicates the shebang, but could work. It would have a cool benefit that you could do this:
Copy code
shell_tests(shells=['bash', 'sh'])
Then Pants will run with all of those shells. That's actually pretty neat! Users could leave off the shebang, so no duplication
Semi-related, maybe 
shell_tests
is a misnomer and it should be 
shunit2_tests
. This target is pretty specific to the shunit2 framework
It would have a cool benefit that you could do this. Then Pants will run with all of those shells.
That could be cool to add to Shellcheck too, which also has a notion of which shell to run with, but has its own mechanisms already by parsing the shebang and falling back to a global option
e
I'm really not following the concern. A shebang line is really easy to either parse or else obviate by sticking your own shebang above it.
But easiest is just 
my/choice/shell unit_test.sh
h
But easiest is just my/choice/shell unit_test.sh
My concern is how do we know what 
my/choice/shell
should be? 1. Parse the shebang line 2. A field on the target I'm thinking #2 because it allows the user to specify multiple shells. I'm looking at shunit2's own repo, and indeed they do that pattern of calling 
my/shell unit_test.sh
over the same file with each shell they care about. Pants can automate that. That's cool
e
Ok. I have no opinion there. I thought the question was about technical difficulties.
h
More modeling difficulties. What makes sense to model, what features do we want to expose, should we support shunit2 in the first place
e
Well, assuming we're all gunning for BUILD files as a last resort, parsing a shebang, taking the basename as indicative, searching for that binary on the (configured) PATH, then executing 
found/binary test.sh
seems like the base answer. Adding BUILD / config metadata to override or steer that for nonstandard cases seems secondary.
h
Support for multiple shells can very much be a follow up or future feature
Just bash seems fine for now
h
I don't think just bash works - if they specified zsh in their shebang, it's important we respect that. So if we want shunit2, we either need to parse shebangs and/or require a field and ignore the shebang I agree that specifying multiple shells for the same shell_tests target can be a future feature. And that's actually going to require changes to 
core/goals/test.py
to enable 1 TestFieldSet -> n results. Or, we approach it like we do with Python interpreter constraints that you create >1 target per file, with a distinct target per shell (gross)
h
Or detect and fail
"Sorry, zsh not supported yet"
Or just run with whatever is in the shebang
h
@happy-kitchen-89482 any thoughts on naming it 
shunit2_tests
vs 
shell_tests
? I lean towards the former because this is so implementation-specific, e.g. the list of supported shells will be what shunit2 supports. The help message will have specific instructions about how the ingration works. etc
h
Yep, shunit2_tests sounds good
👍 1
I mean, are there other test runners for shell?
but best to be specifc now
h
shellspec
@happy-kitchen-89482 would it make sense to have 
runtime_package_dependencies
with shunit2_tests as well? Have shell code to test that your Python distributions were built correctly? It's relatively trivial to implement We can do in a followup or future feature also
h
Followup
👍 1
but cool idea
c
I’ve been away (easter holiday) and like to just follow your discussions, but can’t help chirping in my view (feel free to disregard) Just observing that you have 
python_tests
rather than 
pytest_tests
… 🙂
h
True! I think I proposed renaming it earlier in the year to that 😀 but a couple users asked we hold off to reduce churn, so it didn't go through
👍 1
Open in Slack
PreviousNext
Powered by