Skip to content

Conversation

@edouard-lopez
Copy link
Contributor

@edouard-lopez edouard-lopez commented Jan 24, 2024

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

@edouard-lopez edouard-lopez force-pushed the feat/detect-malformed-files branch from 5930860 to 7fca022 Compare January 24, 2024 15:29
@jorgebucaran
Copy link
Owner

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

@edouard-lopez edouard-lopez force-pushed the feat/detect-malformed-files branch from 7fca022 to f7cbca0 Compare January 24, 2024 16:27
source ./functions/fishtape.fish; fishtape ./tests/invalid.fish

see jorgebucaran#60
@edouard-lopez
Copy link
Contributor Author

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
Copy link
Owner

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

@edouard-lopez
Copy link
Contributor Author

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
Copy link
Owner

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
Copy link
Contributor Author

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

@jorgebucaran
Copy link
Owner

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
Copy link
Contributor Author

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

@edouard-lopez edouard-lopez mentioned this pull request Oct 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants