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

Make asserting failed exit codes and output nicer #336

Merged

Conversation

Chemaclass
Copy link
Member

@Chemaclass Chemaclass commented Sep 24, 2024

📚 Description

Closes: #334

🔖 Changes

  • Allow defer calling a function when asserting exit_code by passing the callable command to evaluate as raw string instead of $(...)
  • bashunit output will be render on the stderr

🖼️ Demo

Render the stdout in the terminal and the errors in a file

Screenshot 2024-09-28 at 10 21 54
export PHPSTAN_PATH=[...]/vendor/bin/phpstan

../bashunit -a exit_code "1" "$PHPSTAN_PATH analyze --no-progress --level 8 --error-format raw ./"
echo $?

../bashunit -a exit_code "0" "$PHPSTAN_PATH analyze --no-progress --level 8 --error-format raw ./"
echo $?

../bashunit -a exit_code "0" "$PHPSTAN_PATH analyze --no-progress --level 8 --error-format raw ./" 2> /tmp/error.log
cat /tmp/error.log

✅ To-do list

  • I updated the CHANGELOG.md to reflect the new feature or fix
  • I updated the documentation to reflect the changes

Screenshot 2024-09-28 at 10 25 43

@Chemaclass Chemaclass added the enhancement New feature or request label Sep 24, 2024
@Chemaclass Chemaclass self-assigned this Sep 24, 2024
@Chemaclass Chemaclass marked this pull request as draft September 24, 2024 20:04
And if there is any output (by a lazy eval as last parameter) that will be rendered into the stdout
echo output to stdout if last arg start with eval on standalone assertions
Adding full control over the stdout and stderr
src/main.sh Outdated Show resolved Hide resolved
@Chemaclass Chemaclass marked this pull request as ready for review September 27, 2024 08:11
@ondrejmirtes
Copy link

I have some ideas about this (which you don't have to follow but maybe just think about the direction):

  1. If you want bashunit -a exit_code "1" "eval the_command" then what would bashunit -a exit_code "1" "the_command" mean? Does it mean anything? Can we remove the "eval" and still have it work?

Does -a exit_code imply the thing in double quotes is going to be run as command?

Also trying to think of some other combinations that either need to mean something, or they need to error.

What would this do?

./bashunit -a assert_same "foo" "eval the_command"

The 2nd argument can just be a normal string. I think special strings shouldn't switch the behaviour of the command.

My preferred syntax would be to have:

bashunit -e "the_command"

It'd mean that bashunit would run the command and then assert what we ask it to assert. Right now the only assertion would be exit code:

bashunit -a exit_code "1" -e "the_command"

Maybe other asserts could work on the output automatically?

bashunit -a exit_code "1" -a contains "some error" -e "the_command" (I don't know if I can currently pass multiple -a args to perform multiple assertions)

Also it'd be nice to save the output to a variable with another option for further asserts:

bashunit -a exit_code "1" -e "the_command" -o $OUTPUT

@Chemaclass
Copy link
Member Author

Chemaclass commented Sep 27, 2024

I like your proposal of removing eval from the second string passed to the exit_code:

  • bashunit -a exit_code "1" - compare with the last exit code
  • bashunit -a exit_code "1" "the_command" - run the command inside bashunit and compare the codes
  • bashunit -a exit_code "1" "$(the_command)" - run the command outside and compare the codes

Basically, the change I would suggest/do is to keep the same behavior (no need to introduce -e or -o) and only for the exit_code. If the second argument is a string, then treat it as a callable (which will run internally with native bash eval).


PS: after some rethinking, I find these two ideas very interesting. I will investigate deeper into what possibilities do we have to get something like this, but in a follow up PR to not overcomplicate/delay this one.

Ideas to investigate

  1. to save the output to a variable with another option for further asserts:
    bashunit -a exit_code "1" "the_command" -o $OUTPUT
    More: Add output option to store in variable #338

  2. Allow multiple -a when using bashunit standalone
    `bashunit -a exit_code "1" "the_command" -a contains "some error"
    More: Allow multiple asserts on standalone exit_code assertion #339


I would discard (at least for now) the -e option because this is part of the current behavior of:

@Chemaclass Chemaclass force-pushed the feat/334-make-asserting-failed-exit-codes-and-output-nicer branch from 64532a3 to 8abe438 Compare September 28, 2024 08:30
@Chemaclass Chemaclass merged commit 6768abc into main Sep 28, 2024
8 checks passed
@Chemaclass Chemaclass deleted the feat/334-make-asserting-failed-exit-codes-and-output-nicer branch September 28, 2024 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Make asserting failed exit codes and output nicer
2 participants