Skip to content

Add annotation to skip all but first test #518

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

Closed

Conversation

matthewmorgan
Copy link

@matthewmorgan matthewmorgan commented Sep 3, 2017

I'm not sure if this behavior is desired, but it's pretty common in the other exercism tracks I've seen:

1. Test suite arrives to the user with only the first test enabled. All others are skipped.
2. After getting the first test passing, user 'unskips' the next test.
3. pass test, rinse, repeat...

I implemented the change in check_exercises.py to 'unskip' the skipped tests for CI.

If this isn't wanted, no big deal, I'll just close it. LMK!

Closes #391

NB I made a change to this to use Python file methods instead of calling sed.

@matthewmorgan matthewmorgan force-pushed the skip-isogram-tests branch 4 times, most recently from f8dc45d to 1df4f7b Compare September 3, 2017 21:14
@matthewmorgan matthewmorgan changed the title Add skip annotation to skip all but first test Add annotation to skip all but first test Sep 3, 2017
@matthewmorgan matthewmorgan force-pushed the skip-isogram-tests branch 2 times, most recently from 8db6dae to 00944b3 Compare September 3, 2017 21:27
@matthewmorgan
Copy link
Author

@m-a-ge @behrtam anything you want changed on this PR, or do you intend to merge it as is?

@behrtam
Copy link
Contributor

behrtam commented Sep 6, 2017

I don't think this is a good idea. This just pollutes the tests and asks for manual changes of the test suit. This is something that should instead be handled by tools like pytest -x isogram_test.py (fail fast after first) which is already described in the docs.

@matthewmorgan
Copy link
Author

@behrtam thanks for your response. I respectfully disagree, and hope you'll consider my point of view.

I'm surprised that you're against the idea of skipping the tests, as it's a common pattern on other exercism tracks (Ruby, Java, JavaScript, C++, EcmaScript for example,) and (IMO) a fundamental piece of the TDD workflow, which is one of the core principles of exercism.

Be that as it may, I do see an open issue in your backlog to 'unskip' optional tests for certain suites. The changes I made to check_exercises.py would accomplish that. If you would prefer to see that change in a separate PR, just let me know and I can do that instead.

@behrtam
Copy link
Contributor

behrtam commented Sep 7, 2017

I'm not "against the idea of skipping the tests" quite the contary I love the TDD flow (red - green - refactor). I'm just against polluting the test suit to do this when there are tools that can do this cleaner and nicer for you.

@matthewmorgan
Copy link
Author

@behrtam I understand. Do you want me to put the check_exercises.py charges in another PR, or modify this one, or are you not interested in those changes either?

@matthewmorgan
Copy link
Author

@behrtam on thinking about this further: I don't agree that adding the skip annotations 'pollutes' the test suite. Quite the contrary-- I think it is declarative, and conveys with better clarity to the user the intention that the tests should be approached and passed one by one.

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

Successfully merging this pull request may close these issues.

Run skipped tests
2 participants