Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace [<TestCase>] tests with individual tests #298

Closed
ErikSchierboom opened this issue Feb 7, 2017 · 10 comments
Closed

Replace [<TestCase>] tests with individual tests #298

ErikSchierboom opened this issue Feb 7, 2017 · 10 comments

Comments

@ErikSchierboom
Copy link
Member

We have several places where we use [TestCase] to create a single test with multiple input values. While efficient (no code duplication), this is not the best of user experiences as it makes it harder to run a single test and also doesn't really allow us to mark the various input values with a descriptive text. It also doesn't align with how canonical data for exercises is defined, which only works on single test cases.

Consider the isogram exercises's tests:

[<TestCase("duplicates", ExpectedResult = true)>]
[<TestCase("eleven", ExpectedResult = false, Ignore = "Remove to run test case")>]
[<TestCase("subdermatoglyphic", ExpectedResult = true, Ignore = "Remove to run test case")>]
[<TestCase("Alphabet", ExpectedResult = false, Ignore = "Remove to run test case")>]
[<TestCase("thumbscrew-japingly", ExpectedResult = true, Ignore = "Remove to run test case")>]
[<TestCase("Hjelmqvist-Gryb-Zock-Pfund-Wax", ExpectedResult = true, Ignore = "Remove to run test case")>]
[<TestCase("Heizölrückstoßabdämpfung", ExpectedResult = true, Ignore = "Remove to run test case")>]
[<TestCase("the quick brown fox", ExpectedResult = false, Ignore = "Remove to run test case")>]
[<TestCase("Emily Jung Schwartzkopf", ExpectedResult = true, Ignore = "Remove to run test case")>]
[<TestCase("éléphant", ExpectedResult = false, Ignore = "Remove to run test case")>]
let ``Isogram correctly detects isograms`` (actual: string) =
    isogram actual

What's the difference between test case one and two? It's not immediately obvious. The canonical-data of this exercise looks much nicer. I think we should convert the [] tests to individual tests.

@robkeim
Copy link
Contributor

robkeim commented Feb 7, 2017

This is also being discussed in the C# track exercism/csharp#194

@robkeim
Copy link
Contributor

robkeim commented Feb 7, 2017

@ErikSchierboom
Copy link
Member Author

Great work gathering this data @robkeim!

@balazsbotond
Copy link
Contributor

I can start working on these. I will always leave a comment here before starting to convert the tests for an exercise - this way you'll know which ones are still up for grabs :)

@robkeim
Copy link
Contributor

robkeim commented Feb 8, 2017

Sounds great, thanks @balazsbotond.

@ErikSchierboom
Copy link
Member Author

ErikSchierboom commented Feb 8, 2017

That would be brilliant @balazsbotond!

@balazsbotond
Copy link
Contributor

balazsbotond commented Feb 9, 2017

Done:

  • acronym
  • atbash-cipher
  • clock
  • isogram
  • luhn
  • nth-prime

robkeim pushed a commit that referenced this issue Feb 9, 2017
#300)

* Fix #286 - Linked List: change pop and shift to return option

* Linked List: Add missing Ignore attribute

* Markdown: Use correct tags for italic and bold text

* Revert "Fix #286 - Linked List: change pop and shift to return option"

This reverts commit 9da0115.

* atbash-cipher: Replace [<TestCase>] tests with individual tests (#298)

* Add missing [<Test>] attribute

* Add missing [<Ignore>] attributes
robkeim pushed a commit that referenced this issue Feb 9, 2017
* Fix #286 - Linked List: change pop and shift to return option

* Linked List: Add missing Ignore attribute

* Markdown: Use correct tags for italic and bold text

* Revert "Fix #286 - Linked List: change pop and shift to return option"

This reverts commit 9da0115.

* Acronym: Replace [<TestCase>] tests with individual tests (#298)

* acronym: Add missing [<TestCase>] attribute

* Add missing [<Ignore>] attributes
robkeim pushed a commit that referenced this issue Feb 9, 2017
* Fix #286 - Linked List: change pop and shift to return option

* Linked List: Add missing Ignore attribute

* Markdown: Use correct tags for italic and bold text

* Revert "Fix #286 - Linked List: change pop and shift to return option"

This reverts commit 9da0115.

* clock: Replace [<TestCase>] tests with individual tests (#298)
@stale stale bot added the wontfix label Apr 29, 2017
@stale
Copy link

stale bot commented Apr 29, 2017

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@robkeim
Copy link
Contributor

robkeim commented Nov 18, 2017

@ErikSchierboom I'm closing this issue as this should now be obsolete with the move to using test generators.

@robkeim robkeim closed this as completed Nov 18, 2017
@ErikSchierboom
Copy link
Member Author

@robkeim Good point.

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

No branches or pull requests

3 participants