elm-explorations / test

Write unit and fuzz tests for Elm code.
https://package.elm-lang.org/packages/elm-explorations/test/latest
BSD 3-Clause "New" or "Revised" License
237 stars 39 forks source link

Aggregate multiple duplicate descriptions at the same level #163

Closed mpizenberg closed 3 years ago

mpizenberg commented 3 years ago

Hi, following suggestions that it would be nicer to have all duplicates at once, I've come up with a possible implementation of this. Given the following tests:

concatDup : Test
concatDup =
    Test.concat
        [ Test.test "should pass" (\_ -> Expect.pass)
        , Test.test "should pass" (\_ -> Expect.pass)
        , Test.test "should fail" (\_ -> Expect.fail "force failed")
        , Test.test "should fail" (\_ -> Expect.fail "force failed")
        ]

suite : Test
suite =
    Test.describe "suite"
        [ Test.test "should pass" (\_ -> Expect.pass)
        , Test.test "should pass" (\_ -> Expect.pass)
        , Test.test "should fail" (\_ -> Expect.fail "force failed")
        , Test.test "should fail" (\_ -> Expect.fail "force failed")
        ]

anotherSuite : Test
anotherSuite =
    Test.describe "another suite"
        [ Test.test "another suite" (\_ -> Expect.pass)
        ]

Instead of getting the following elm-test output:

✗ Tests

    The test 'another suite' contains a child test of the same name. Let's rename them so we know which is which.

✗ Tests

    A test group contains multiple tests named 'should pass'. Do some renaming so that tests have unique names.

✗ Tests

    The tests 'suite' contain multiple tests named 'should pass'. Let's rename them so we know which is which.

We are getting the following one:

↓ Tests
✗ another suite

    The test 'another suite' contains a child test of the same name. Let's rename them so we know which is which.

✗ Tests

    A test group contains multiple tests named 'should fail'. Do some renaming so that tests have unique names.
    A test group contains multiple tests named 'should pass'. Do some renaming so that tests have unique names.

↓ Tests
✗ suite

    Contains multiple tests named 'should fail'. Let's rename them so we know which is which.
    Contains multiple tests named 'should pass'. Let's rename them so we know which is which.

You can notice that in the case of concat and describe, both "should fail" and "should pass" duplicates are reported. And in the case of describe and of duplicate child, the labels go one step further.

To gather all duplicates, I simply aggregate them inside a Set instead of failing fast with a Result.andThen in fold.

Side note, I had to remove the Elm.Kernel.Debug import to be able to compile, I'm not sure why.

harrysarson commented 3 years ago

Side note, I had to remove the Elm.Kernel.Debug import to be able to compile, I'm not sure why.

See https://github.com/elm-explorations/test/pull/145 (its a bug in elm 0.19.1).

harrysarson commented 3 years ago

So if you can revert the Elm.Kernel.Debug changes and then use ./tests/make.sh to compile rather than elm make everything should work :)

harrysarson commented 3 years ago
[{"path":"benchmarks/Main.elm","message":"File is not formatted with elm-format-0.8.5 --elm-version=0.19"}
]

CI failures :)

harrysarson commented 3 years ago

@avh4 @drathier any objections landing this on master and queuing it for the 2.0 release (whenever that should happen)?

drathier commented 3 years ago

Sorry, I've been away. Looks fine to me.

harrysarson commented 3 years ago

ping @mpizenberg. Just waiting on elm-format so we can merge :)

harrysarson commented 3 years ago

re-ping @mpizenberg :)

mpizenberg commented 3 years ago

I had forgotten about this ^^. Is there anything I have to do?

mpizenberg commented 3 years ago

Oh I have to elm-format the code? Let me look at this later today!

mpizenberg commented 3 years ago

Since it was kinda unrelated, I opened another PR for formatting in #170. Then the CI here should pass.

mpizenberg commented 3 years ago

@harrysarson it does not seem like the close/open trick will do it ^^. Maybe we wait for the switch to GH Action in #173 ?

image

mpizenberg commented 3 years ago

Let me rebase this.