-
Notifications
You must be signed in to change notification settings - Fork 9.8k
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
test: Add missing test case for from_str exercise #368
Conversation
Hi @apatniv, Thanks for offering to improve the test cases for the I believe incorporating those changes would go hand in hand with this pull request. Please feel free to reach out if you have any additional questions. Thank you, Abdou |
Abdou Thanks for your response. I will take a look at #358 and add the necessary checks. |
Would you recommend putting all malformed strings into one test case like this
or create a new test function for each of the cases like
Former is easier to write but may be bad for reading/debugging purpose. Later, is easier to debug at the expense of verbosity as we need to add similar test cases in IMPO, later approach may be easier for beginners. |
Hi @apatniv, I am much more drawn to the second option for two reasons:
I understand that it's going to be quite verbose, so feel free to do whatever sounds better to you and does not consume too much of your precious time. Thank you, Abdou |
@AbdouSeck Thanks for the feedback. I add all the possible test cases and tried to follow the conventions in individual test cases. |
Hi @apatniv, this looks fantastic! Thank you so much for the work! Thank you all! Abdou |
Thanks! |
@all-contributors add @apatniv for code and test |
@fmoko I've put up a pull request to add @apatniv! 🎉 |
Added the missing test case for "from_str" exercise.