Skip to content

misleading test description in the forms validators spec #21575

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.

Sign up for GitHub

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

Jump to bottom
Closed
IgorMinar opened this issue Jan 16, 2018 · 5 comments
Closed

misleading test description in the forms validators spec #21575

IgorMinar opened this issue Jan 16, 2018 · 5 comments
Labels
area: forms help wanted An issue that is suitable for a community contributor (based on its complexity/scope). refactoring Issue that involves refactoring or code-cleanup

Comments

@IgorMinar
Copy link
Contributor

angular/packages/forms/test/validators_spec.ts

Lines 274 to 275 in 54bf179

it('should work with string containing line boundary',
() => expect(Validators.pattern('^[aA]*$')(new FormControl('aaAA'))).toBeNull());

where is the line boundary in the test string? I don't see it. I believe this test shows that we don't match when line boundary is missing - it's a good test but the description is wrong, please correct it.

Can you also add the missing test please?

note: original comment
IgorMinar referenced this issue Jan 16, 2018
@ghetolay @alexeagle
feat(forms): handle string with and without line boundary on pattern …
54bf179 
…validator (#19256)

PR Close #19256
@kara
Copy link
Contributor

kara commented Jan 16, 2018 •
edited
Loading

Update: @IgorMinar and I discussed offline. Test description should read:

should work with pattern string containing boundary symbols to clarify

@kara kara changed the title wrong test in the forms validators spec misleading test description in the forms validators spec Jan 16, 2018
@kara kara added refactoring Issue that involves refactoring or code-cleanup help wanted An issue that is suitable for a community contributor (based on its complexity/scope). labels Jan 16, 2018
@ghetolay
Copy link
Contributor

ghetolay commented Jan 16, 2018 •
edited
Loading

I'll do the PR tomorrow but what is the missing test ?

@kara
Copy link
Contributor

kara commented Jan 16, 2018 •
edited
Loading

I don't think there's a missing test. The test description was just confusing because it sounded like it had to do with multiline/single-line regexes instead of "string starts with", etc. It also wasn't clear whether string referred to the test string or the pattern string.

@ghetolay
Copy link
Contributor

Ok sorry for that missunderstanding, I'm a better coder than English speaker :D.

If we were to provide full coverage of the code we could add 2 tests :

  • Only using ^ like ^aaAA
  • Only using $ like aaAA$

ghetolay added a commit to ghetolay/angular that referenced this issue Jan 18, 2018
@ghetolay
test(forms): better description and coverage for angular#19256
b30a93c 
fixes angular#21575
ghetolay added a commit to ghetolay/angular that referenced this issue Jan 18, 2018
@ghetolay
test(forms): Better description and coverage for angular#19256
5086d34 
fixes angular#21575
@ghetolay ghetolay mentioned this issue Jan 18, 2018
Closed
mhevery pushed a commit that referenced this issue Jan 24, 2018
@ghetolay @mhevery
test(forms): Better description and coverage for #19256 (#21652)
6921c20 
fixes #21575

PR Close #21652
jbogarthyde pushed a commit to jbogarthyde/angular that referenced this issue Feb 23, 2018
@ghetolay @jbogarthyde
test(forms): Better description and coverage for angular#19256 (angul…
aeebe45 
…ar#21652)

fixes angular#21575

PR Close angular#21652
leo6104 pushed a commit to leo6104/angular that referenced this issue Mar 25, 2018
@ghetolay @leo6104
test(forms): Better description and coverage for angular#19256 (angul…
48abac8 
…ar#21652)

fixes angular#21575

PR Close angular#21652
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area: forms help wanted An issue that is suitable for a community contributor (based on its complexity/scope). refactoring Issue that involves refactoring or code-cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

test(forms): Better tests description and coverage for #19256 ghetolay/angular
3 participants
@IgorMinar @ghetolay @kara