-
-
Notifications
You must be signed in to change notification settings - Fork 137
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
sync phone-number #803
sync phone-number #803
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 |
Long time no see. I have started this PR. But would like some feedback if I have understood the tasks correctly. I have done following: Testcases with uuid: 2de74156-f646-42b5-8638-0ef1d8b58bc2 and 4a1509b7-8953-4eec-981b-c483358ff531 are not implemented, as they would break current community solutions. So how to deal with those? I don't see which 4 tests cases that aren't implemented according to https://github.com/exercism/problem-specifications/blob/main/exercises/phone-number/canonical-data.json . |
Hi Tomas, nice to see you contributing again. Please don't use /**
* uuid 79666dce-e0f1-46de-95a1-563802913c35
* @testdox Cleans the number
*/
The
This means, UUID
Please describe: Why are those test cases breaking solutions? Did some rule change? Or did some untested edge-case slip through? How many solutions exist (see track build status -> Practice Exercises -> active practice exercises -> column "completions")? How many of the first page of the community solutions would break? |
Thanks. Happy to be back.
Done.
This is the output from the The `phone-number` exercise has up-to-date docs, filepaths, metadata, and tests!
There expected error messages has change, from "incorrect number of digits" to "must not be fewer than 10 digits" and as we are checking for $this->expectExceptionMessage('incorrect number of digits'); Therefor the tests will break. The same goes for the other test case, not updated. It's just different wordings. 181 persons have completed that exercise. To my knowledge all will break if changing the wording in the exception messages. |
Thank you for adjusting the DocBlocks. Don't forget to un-draft the PR, so it can be merged. For compatibility remove testing for the exception message or use |
Will do one of the next days. Don't have time right now. But will take care asap. |
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.
Thanks for your work! I think expectExceptionMessageMatches()
needs the RegExp to be surrounded by limiters (e.g. /.* than 11 digits/
). But I always have to try that out myself 😃
Solves issue #800