https://pantsbuild.org/ logo
#development
Title
# development
h

hundreds-father-404

04/02/2021, 5:22 PM
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

enough-analyst-54434

04/02/2021, 5:44 PM
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

hundreds-father-404

04/02/2021, 5:47 PM
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

enough-analyst-54434

04/02/2021, 5:49 PM
Ok. I have no opinion there. I thought the question was about technical difficulties.
h

hundreds-father-404

04/02/2021, 5:50 PM
More modeling difficulties. What makes sense to model, what features do we want to expose, should we support shunit2 in the first place
e

enough-analyst-54434

04/02/2021, 6:16 PM
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

happy-kitchen-89482

04/02/2021, 6:29 PM
Support for multiple shells can very much be a follow up or future feature
Just bash seems fine for now
h

hundreds-father-404

04/02/2021, 6:33 PM
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

happy-kitchen-89482

04/02/2021, 6:37 PM
Or detect and fail
"Sorry, zsh not supported yet"
Or just run with whatever is in the shebang
h

hundreds-father-404

04/02/2021, 6:44 PM
@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

happy-kitchen-89482

04/02/2021, 6:45 PM
Yep, shunit2_tests sounds good
👍 1
I mean, are there other test runners for shell?
but best to be specifc now
h

hundreds-father-404

04/02/2021, 6:46 PM
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

happy-kitchen-89482

04/05/2021, 5:46 PM
Followup
👍 1
but cool idea
c

curved-television-6568

04/08/2021, 6:51 AM
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

hundreds-father-404

04/08/2021, 7:22 AM
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