Aggregate multiple duplicate descriptions at the same level

[mpizenberg]

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]

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

See #145 (its a bug in elm 0.19.1).

[harrysarson]

This wrapper Test constructor is new right? What is the effect of adding it/what was missing before?

[harrysarson]

Ditto for https://github.com/elm-explorations/test/pull/163/files#diff-c69646ee43af4eee9572fe3140c4d710eb5821628e80a081b455cc7583511933R119

[mpizenberg]

And in the case of describe and of duplicate child, the labels go one step further.

That's the effect, you can see it in the example output up in this thread

[harrysarson]

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]

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

CI failures :)

[harrysarson]

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

[drathier]

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

[harrysarson]

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

[harrysarson]

re-ping @mpizenberg :)

[mpizenberg]

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

[mpizenberg]

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

[mpizenberg]

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

[mpizenberg]

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

[mpizenberg]

Let me rebase this.