jorgebucaran / fishtape

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

Unsupported -a in fishtape 2.0.0 #40

Closed matchai closed 5 years ago

matchai commented 5 years ago

Great work with the release of fishtape 2.0.0!

Unfortunately, the new test system doesn't allow for multiple assertions in one test, so the test setup would have to be duplicated for every test case.

E.g:

@test "It should not mock blacklisted elements" (
    mock eval \*
) = "The function \"eval\" is reserved and therefore cannot be mocked."

@test "It should not mock blacklisted elements" (
    mock eval \*
) $status -eq 1

instead of:

@test "It should not mock blacklisted elements" (
    mock eval \*
) = "The function \"eval\" is reserved and therefore cannot be mocked." -a $status -eq 1

In this example, the test setup is short, but a fair bit of duplication would need to be added tests where setup is more complex, without a way to have multiple assertions.

jorgebucaran commented 5 years ago

@matchai Funny story. Fishtape 1 didn't have this either. Your tests will pass even if the next condition fails.

matchai commented 5 years ago

@jorgebucaran Oh... That's a shock. 😳 I guess I'll be quickly updating my tests in that case.

jorgebucaran commented 5 years ago

@matchai What happens if the expression fails—how do we report that with TAP?


Here is an alternative. Print the $status right after running mock:

@test "It should not mock blacklisted elements" (
    echo (
        mock eval \*
        echo $status
    )
) = "The function \"eval\" is reserved and therefore cannot be mocked. 0"

We need to wrap mock and echo $status in another echo to remove new lines.

Here is another idea: If you check the exit status often, create a utility function to echo stdout if $staus is 0.

function only_if_ok
    and echo "$argv"; or echo
end

@test "It should not mock blacklisted elements" (
    only_if_ok (mock eval \*)
) = "The function \"eval\" is reserved and therefore cannot be mocked."
matchai commented 5 years ago

What happens if the expression fails—how do we report that with TAP?

I think the ideal thing would be to support subtests. Though they have never been standardized in TAP, they're commonly used. (https://github.com/TestAnything/Specification/issues/2)


Both solid suggestions! I'll try them out. 👍

jorgebucaran commented 5 years ago

@matchai I disagree.

I'm not opposed to subtests (see below), but I am positive that using conditional operators (-a or -o) to describe them would be the wrong tool for the job.

Now, how do we describe subtests using our limited syntax (jorgebucaran/fishtape#34)?

Here's an idea that doesn't rely on syntax:

fishtape parent_test.fish
# Parent
    # Child
        # Grandchild
            ok 1 - some foo test
            ok 2 - some bar test
            not ok 3 - some baz test
            not ok 3 - some baz test
              ---
                operator: =
                expected: bar
                actual:   baz
              ...
            1..3
            # tests 3
            # pass  2
            # fail  1
        ok 1 - grandchild_test.fish
        1..1
        # tests 1
        # fail  1
    ok 1 - child_test.fish
    1..1
    # tests 1
    # fail  1
ok 1 - parent_test.fish
1..1
# tests 1
# fail  1
parent_test.fish
@mesg "Parent"
fishtape child_test.fish
child_test.fish
@mesg "Child"
fishtape granchild_test.fish
grandchild_test.fish
@mesg "Grandchild"
test "some foo test" foo = foo
test "some bar test" bar = bar
test "some baz test" bar = baz

Closing as I won't be implementing conditional operators at this time, but we can discuss subtests in a separate issue.

matchai commented 5 years ago

I am positive that using conditional operators (-a or -o) to describe them would be the wrong tool for the job

Oh yeah, absolutely. I was just thinking that having multiple test cases share the same setup would be best done with subtests. If it would be possible for tests to inherit their parents' setup, that would make -a or -o unnecessary.

jorgebucaran commented 5 years ago

@matchai Ah, I see what you mean then. Glad we're clear about that.