London|26-ITP-January|Alexandru Pocovnicu|Sprint 3 |stretch#951
London|26-ITP-January|Alexandru Pocovnicu|Sprint 3 |stretch#951alexandru-pocovnicu wants to merge 17 commits intoCodeYourFuture:mainfrom
Conversation
…bers, english letters upper/lower case , non-alphanumeric symbols and it has a length of minimum 5 letters
…nd-rewrite branch
This comment has been minimized.
This comment has been minimized.
alexandru-pocovnicu
added
the
Needs Review
This comment has been minimized.
This comment has been minimized.
github-actions
bot
removed
the
Needs Review
This comment has been minimized.
This comment has been minimized.
3 similar comments
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
alexandru-pocovnicu
changed the title
alexandru-pocovnicu
added
the
Needs Review
There was a problem hiding this comment.
When one of these tests fails, these test descriptions do not quite tell the developer why their function failed.
Can you make the test descriptions on this file more informative?
There was a problem hiding this comment.
These three test descriptions are greatly improved! Why not also update the remaining test descriptions?
cjyuan
added
Reviewed
|
thank you |
alexandru-pocovnicu
added
Needs Review
There was a problem hiding this comment.
When this test fails, it only means the function fails to recognise a valid password -- which may or may not related to uppercase letters.
I think you can consider grouping all valid cases into the same category.
There was a problem hiding this comment.
This sample does not satisfy the criterium: it meets all other conditions except the one you're targeting.
There are a few more test samples in this file that fit this description.
Can you update them accordingly?
cjyuan
added
Reviewed
|
thank you, I'm still not sure about checking if the password has been used before, at the moment it's just checking against an empty array |
alexandru-pocovnicu
added
Needs Review
cjyuan
left a comment
cjyuan
left a comment
There was a problem hiding this comment.
Changes look good. Well done!
I'm still not sure about checking if the password has been used before, at the moment it's just checking against an empty array
What are you unsure about?
| test("should return true if at least one of the digits isn't the same as the others",()=>{ | ||
| expect(validateNumber(3636363636363636)).toEqual(true) | ||
| }) | ||
| test("should return true if at least one of the digits isn't the same as the others", () => { | ||
| expect(validateNumber(3333333333333336)).toEqual(true); | ||
| }); | ||
| test("should return false if at least one of the digits isn't different from all the others", () => { | ||
| expect(validateNumber(2222222222222222)).toEqual(false); | ||
| }); |
There was a problem hiding this comment.
Suggestion: Ask an AI tool how these test descriptions can be made more concise.
No change needed. 3
cjyuan
added
Complete
2 participants
Learners, PR Template
Self checklist
Changelist
set up script and test files for card validator, done tdd to check if a password and credit card are valid
##Questions
got stuck on how to check if a password was used before , don't really know where or what to look for