Make asserting failed exit codes and output nicer

[ondrejmirtes]

Creating this based on our discussion in Dresden :)

If we want to assert that a command failed & assert its output, we have to do something like this:

OUTPUT=$(../../bin/phpstan analyze --no-progress --level 8 --error-format raw data/ || true)
echo "$OUTPUT"
../bashunit -a line_count 1 "$OUTPUT"
../bashunit -a contains 'Method TraitsCachingIssue\TestClassUsingTrait::doBar() should return stdClass but returns Exception.' "$OUTPUT"

The problem is that we're not really asserting that the commands returned exit code greater than zero, and also that we need to do the whole OUTPUT=$(command || true) dance.

Would be great if Bashunit provided a nicer way to do this :)

[Chemaclass]

Sure thing! @ondrejmirtes, can you give me an example of how would you vision the ideal use case? Especially, how would you use bashunit in this context (what would you like to have). This can help me going in the direction you need (test-first mindset), because right now I am not sure if I got the solution/problem right.
Also, if you can give me a link of a real example of this in your CI that might help too (to contextualize even more)

The core issue, I think, is that you currently need the || true because ../../bin/phpstan analyze have an exit_code !=0 and you are piping to make sure the error output is store in the OUTPUT variable? or something else? If this is the case, I am wondering how could bashunit help in this context... especially considering that you still want to assert that the line_count=1 and contains="Method ..." 🤔 I mean, you would still need to save the OUTPUT variable there, or? it might be that this is a design flaw of bash itself. I am trying to understand how different would it be to make it simpler than it is right now, so we can see how can we help you on this one // CC @staabm

[Chemaclass]

I am playing around trying to feel the pain/problem

<?php
class Testing {
  function bar(int $baz = 0) {
    return $baz;
  }
}

# Current example
OUTPUT=$("$PHPSTAN_PATH" analyze --no-progress --level 8 --error-format raw local/ || true)
echo "$OUTPUT"
./bashunit -a line_count 1 "$OUTPUT"
./bashunit -a contains 'Method teseting::bar() has no return type specified.' "$OUTPUT"

# Alternative(?)
OUTPUT=$("$PHPSTAN_PATH" analyze --no-progress --level 8 --error-format raw local/)
./bashunit -a line_count 1 "$OUTPUT"
./bashunit -a contains 'Method teseting::bar() has no return type specified.' "$OUTPUT"

Both example works just fine. This is why I mention I feel I need more context, because using the Alternative()? feels right to me, so most luckily I am missing something here :)

[staabm]

there is still no assertion for the exit code of the phpstan command

[Chemaclass]

@staabm you can assert the exit codes as well: https://bashunit.typeddevs.com/assertions#assert-exit-code
or do you mean that the phpstan command is not returning a 1 exit code for general errors?

[staabm]

The github action will stop executing further commands as soon as a commands exits with non-zero.

This means you can't assert after a command anymore as the execution already stopped
(Similar to a bash script executed with set -e).

I need to test it.. give me a few hours ;-)

[Chemaclass]

Have you tried continue-on-error: true. Eg:

      - name: Run PHPUnit tests
        run: phpunit --testdox
        continue-on-error: true

Docs: https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions

Although, this might be similar as doing this ... 🤔

      - name: Run PHPUnit tests
        run: phpunit --testdox || true

[staabm]

so our use case is, that we want to assert the output and also the exit code of the phpstan command

we could assert the exit code with - I guess:

../bashunit -a exit_code "1" "$(../../bin/phpstan analyze --no-progress --level 8 --error-format raw data/)"

but not the output.

or we can use

OUTPUT=$(../../bin/phpstan analyze --no-progress --level 8 --error-format raw data/ || true)
./bashunit -a line_count 1 "$OUTPUT"

to assert the output.. but we cannot check the exit code that way

[staabm]

I think something like this could work:

OUTPUT=$(./bashunit --forward-stdout -a exit_code "1" "$(../../bin/phpstan analyze --no-progress --level 8 --error-format raw data/)")
./bashunit -a line_count 1 "$OUTPUT"

in case we could instruct bashunit to pass the stdout of the command thru.
same could work for --forward-stderr,
or maybe even both at once, in case we add some magic

[Chemaclass]

@ondrejmirtes @staabm how does this looks like for you? Here you have the MVP based on your last comment, @staabm

I think this deferrable idea looks very sexy, however I am not sure about callable:(...) [instead of $(...)], maybe you have other suggestions? 🤔

This POC is now only working on for the standalone assert_exit_code but I wonder if this use case might be beneficial in other assertions or other places. Maybe you can help me with this thought, what do you think about this so far? I will work tomorrow in adding tests 🧪

[staabm]

I would not mix the output of bashunit and the callable on the same output stream.

Bashunit output should go to stderr and the callable output to stdout

The bashunit command should return exit code 0 if the assertion matched and otherwise non-zero

All of the above only when --forward-stdout. Maybe we don't need the option at all, because bashunit will already behave correctly without it. I am not sure.

I don't think we need it for other assertions, because after output has been caputred via stdout into a variable, I can use any assertion on said variable

[Chemaclass]

@ondrejmirtes @staabm yes, that make sense. bashunit will render on stderr, and if you provide a eval script as last param then it will render its output on the stdout. Btw, I changed that callable:(...) thing to simply use eval ...

By default stderr and stdout render to the terminal on the same way, but you can redirect the FD wherever you want. No need for the --forward-stdout.

How does this looks like now? #336