Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

shell: add experimental_test_shell_command for arbitrary shell-driven tests #17640

Merged
merged 5 commits into from
Nov 26, 2022

Conversation

tdyas
Copy link
Contributor

@tdyas tdyas commented Nov 26, 2022

Add an experimental_test_shell_command target type to enable defining arbitrary tests using shell code.

Note: This target is not a unit test of shell sources like shunit2_tests; rather, it is a test defined in shell which could test anything which can be packaged and consumed by the other shell targets (e.g., experimental_shell_command and experimental_run_shell_command). It is basically experimental_run_shell_command but for the test goal instead of the run goal.

@tdyas tdyas added category:new feature backend: Shell Shell backend-related issues labels Nov 26, 2022
@tdyas tdyas requested review from Eric-Arellano, stuhood, chrisjrn and benjyw and removed request for stuhood November 26, 2022 03:33
@tdyas
Copy link
Contributor Author

tdyas commented Nov 26, 2022

Example from pants-go-testing: tdyas/pants-go-testing@64a0d49

@tdyas tdyas force-pushed the shell_test_support branch from 9cc7c4e to 679b5a9 Compare November 26, 2022 04:00
Copy link
Contributor

@benjyw benjyw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super-neat! Not to bikeshed, but I had a suggestion re name.

I'll also let @chrisjrn weigh in, since he's been looking at some of this.

@@ -393,6 +403,42 @@ class ShellCommandRunTarget(Target):
)


class ShellCommandTestTarget(Target):
alias = "experimental_test_shell_command"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name is a bit of a mouthful. I'm wondering about naming it experimental_generic_test? The "shell" name here (and in experimental_shell_command) is a bit misleading, since this can run arbitrary processes, it's not limited to just shell constructs. And it's also easily confused with "tests shell code".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have a particular care as to what the name is. Please feel free to just tell me what the name should be.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also if called generic_test, should it be in a separate generic backend instead of the shell backend?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can leave as-is for now, since we have to rename experimental_shell_command anyway before we de-experimentalize it.

Copy link
Member

@kaos kaos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. I've no strong opinions on naming this either..

Perhaps the _command suffix could do without, so experimental_test_shell ?

I also like the generic_test bit 🤷🏽

src/python/pants/backend/shell/goals/test_test.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend: Shell Shell backend-related issues category:new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants