Skip to content
This repository has been archived by the owner on Oct 9, 2019. It is now read-only.

Fail tests with duplicate names #133

Merged
merged 4 commits into from
Mar 3, 2017
Merged

Fail tests with duplicate names #133

merged 4 commits into from
Mar 3, 2017

Conversation

mgold
Copy link
Member

@mgold mgold commented Feb 26, 2017

Implements #115: automatically fail tests with duplicated names across parents/children and siblings. Test that this works. Rename a pair of tests in our suite that broke this rule.

👀 @rtfeldman @jfmengels

@mgold mgold mentioned this pull request Feb 26, 2017
10 tasks
@jfmengels
Copy link

Awesome! Thanks @mgold :)

@rtfeldman
Copy link
Member

Woo, awesome! 😻

describe "a describe with the same name as a child describe fails"
[ describe "a describe with the same name as a child describe fails"
[ test "a test" passingTest ]
]
Copy link
Member

@rtfeldman rtfeldman Feb 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about sibling tests?

, expectToFail <|
    Test.concat
        [ describe "a describe with the same name as a sibling describe fails"
            [ test "a test" passingTest ]
        , describe "a describe with the same name as a sibling describe fails"
            [ test "another test" passingTest ]
        ]
, expectToFail <|
    Test.concat
        [ Test.concat
            [ describe "a describe with the same name as a de facto sibling describe fails"
                [ test "a test" passingTest ]
            ]
        , describe "a describe with the same name as a de facto sibling describe fails"
            [ test "another test" passingTest ]
        ]
, expectToFail <|
    Test.concat
        [ Test.concat
            [ describe "a describe with the same name as a de facto sibling describe fails"
                [ test "a test" passingTest ]
            ]
        , Test.concat
            [ describe "a describe with the same name as a de facto sibling describe fails"
                [ test "another test" passingTest ]
            ]
        ]

, reason = Test.Expectation.Invalid Test.Expectation.DuplicatedName
}
else
Internal.Labeled desc (Internal.Batch tests)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Test.concat needs to run duplicatedName too.

https://github.com/elm-community/elm-test/pull/133/files#r103121485 would fail if it's only on describe!

@mgold
Copy link
Member Author

mgold commented Feb 28, 2017

@rtfeldman New commit adds your proposed tests and implementation to make them pass.

Because tests is not needed in the closure, declare these functions once.
@mgold
Copy link
Member Author

mgold commented Mar 3, 2017

@rtfeldman What about now?

@rtfeldman
Copy link
Member

Love it! 😻

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants