jorgebucaran / fishtape

100% pure-Fish test runner
MIT License
347 stars 20 forks source link

fix: increase _fishtape_test_failed when file is invalid #66

Open edouard-lopez opened 9 months ago

edouard-lopez commented 9 months ago

related: #60

Here, I reproduce a case of silent error I got on pure due to an invalid file. The change was made through Github UI and suggestion feature so no linting was done.

Test

source ./functions/fishtape.fish; fishtape ./tests/invalid.fish

CI

Not sure how to run this on CI

jorgebucaran commented 9 months ago

-z expects a string. Do we want your test to fail when no string is provided to -z?

edouard-lopez commented 9 months ago

I updated the test with a tautology. The invalid file should be considered a failed test, so the pipeline doesn't succeed.

Below, I highlighted the output changes

❯ source ./functions/fishtape.fish; fishtape tests/invalid.fish                                                                                                                                  ✖️
TAP version 13
~/projects/contributions/fishtape/tests/invalid.fish (line 4): 'end' outside of a block
end # we want to trigger a parsing error
^~^
warning: Error while reading file /home/edouard/projects/contributions/fishtape/tests/invalid.fish

1..0
# pass 0
-# ok
+# fail 1
jorgebucaran commented 9 months ago

Isn't that the same situation with -z? = expects two strings.

edouard-lopez commented 9 months ago

The @test is not important, but an invalid file should trigger a failed exit code so pipeline jobs fails too and are visible as such.

jorgebucaran commented 9 months ago

Are we not currently failing tests when the user misuses -z or =? Does this mean some tests are passing even when a flag is used incorrectly?

edouard-lopez commented 9 months ago

You can check out the result of fishtape tests/invalid.fish with the current Fishtape version, it's not failing.

jorgebucaran commented 9 months ago

I thought the expectation was for incorrect flag usage to result in a failure.

I can see invalid.fish ends with an end that's not valid in Fishtape, correct?

edouard-lopez commented 9 months ago

Yep, it's on purpose, so the file is invalid in Fish and it triggers a parsing error.

Previously, Fishtape didn't increment the _fishtape_test_failed variable in such case and thus resulted in a success (despite the existence of an error).

The behaviour I introduce aligns the exit code of Fishtape with the reality of Fish parsing