Add better support for failed not (!) tests

[mattmc3]

String and numeric tests already have a negation test built-in as an operator (-ne, !=, etc), but for file system tests, there's not a specific operator. Instead, testing that a file or directory does not exist requires a two part operator, eg: test ! -f myfile. Fishtape does not output a great fail message for these tests because it doesn't account for the not (!) operator. This PR adds explicit support for not !.

[mattmc3]

Can do! That's an easy fix.

[jorgebucaran]

Can you think of a test that can't be written with the current Fishtape? We used to support !, but it made the code a bit messier so I removed it in 3.0.0. I'm willing to have another look, though!

[mattmc3]

Hmm, maybe I'm not understanding how the file system tests work without a not operator. For example, imagine testing some functionality that cleans up after itself.

@test "cache does't exist yet" ! -d $cachedir
myapp buildcache
@test "build cache succeeded" $status -eq 0
@test "cache exists" -d $cachedir
myapp cleancache
@test "clean cache succeeded" $status -eq 0
@test "cache was cleaned" ! -d $cachedir

[mattmc3]

Just to clarify - fishtape currently executes negative tests, but the output is not properly descriptive of the failed test.

Current:

TAP version 13
# === files ===
ok 1 a directory
not ok 2 a directory
  ---
    operator: -d
    expected: /var/folders/b8/fk8n159x73q078cc1n2c_6jjd1t92m/T/tmp.xhweXyVB
    actual: !
    at: ~/Projects/github.com/jorgebucaran/fishtape/tests/files.fish:8
  ...
ok 3 a regular file
ok 4 nothing to see here

With my PR:

TAP version 13
# === files ===
ok 1 a directory
else not 
not ok 2 a non-existent directory
  ---
    operator: ! -d
    expected: not a directory
    actual: /var/folders/b8/fk8n159x73q078cc1n2c_6jjd1t92m/T/tmp.Xy7sUuuw
    at: ~/Projects/github.com/mattmc3/fishtape/tests/files.fish:8
  ...
ok 3 a regular file
ok 4 a non-existent regular file
ok 5 nothing to see here

1..5
# pass 4
# fail 1

[jorgebucaran]

Thank you! Just change the README where it says that ! is not supported and give me a few days to review this well. 🙏

[jorgebucaran]

set --local operator "!"
@test "matches ! operator" ! = "$operator"

Just a curiosity. I'm not saying that we should support that, though.

[mattmc3]

I can handle that edge case. We only need to support the ! operator for file system tests, so I can add a check for 3 test args before handling the !not operator. If there's 3 args, we're performing a comparison, not a file system check. Then, we don't need to support wrong-headed tests like @test "double neg" ! a != b or @test "why not use !=" ! a = b.

[jorgebucaran]

Could you explain the recent commits? I thought your original code read cleaner.

[mattmc3]

With some minor tweaks based on your feedback, I have a much cleaner implementation of this feature. It's just one additional special case of the if/else for a failed test, and only adds a few lines of code. Thanks for considering this feature.

[mattmc3]

Could you explain the recent commits? I thought your original code read cleaner.

I can revert back if you prefer that, but this version seemed cleaner and less code to me. Stylistically, I can go either way, I care more about the feature than the specifics of the implementation.

[jorgebucaran]

Yes, sorry, let's go back to the other one. 🙇‍♂️

[jorgebucaran]

Merged and released in 3.0.1. Thank you, @mattmc3! ✨💯