-
-
Notifications
You must be signed in to change notification settings - Fork 746
Add @DisplayName to pig latin #3039
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
Conversation
|
This PR touches files which potentially affect the outcome of the tests of an exercise. This will cause all students' solutions to affected exercises to be re-tested. If this PR does not affect the result of the test (or, for example, adds an edge case that is not worth rerunning all tests for), please add the following to the merge-commit message which will stops student's tests from re-running. Please copy-paste to avoid typos. For more information, refer to the documentation. If you are unsure whether to add the message or not, please ping |
| } | ||
|
|
||
| @Test | ||
| @DisplayName("ay is added to words that begin with a vowel") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The text for the @DisplayName should come from the description in the exercise's canonical data. For example, this particular test case, it the description is on line 9, so the text should be word beginning with a. The other @DisplayNames should be updated to match theirs too.
…for each test.
| @Test | ||
| @Disabled | ||
| @DisplayName("word beginning with i") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @Test | |
| @Disabled | |
| @DisplayName("word beginning with i") | |
| @Disabled("Remove to run test") | |
| @Test | |
| @DisplayName("word beginning with i") |
Avoid making unnecessary changes, such as altering the order between @test and @disabled.
Also, make changes across all instances according to the above suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it helps, the overall diff in the Files changed tab should show just the addition of the @DisplayName annotations. There shouldn't be any changes to @Disabled.
.vscode/settings.json
Outdated
| { | ||
| "java.debug.settings.onBuildFailureProceed": true | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems this file was unintentionally added by your IDE.
Please remove it!
| } | ||
|
|
||
| @Test | ||
| @Disabled |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry I should've mentioned earlier, the only exception is the first test - the first test doesn't get @Disabled. The idea is that Exercism uses TDD where students would first try to get the first test working first. Once they have done that, they can remove the tag from the second test and get that working and so on.
| @Disabled |
pull request
This is my first PR ever, so I'd really appreciate some feedback.
In the pig-latin exercise tests, I added the "@DisplayName" annotation to each test.
Reviewer Resources:
Track Policies