-
Notifications
You must be signed in to change notification settings - Fork 482
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
Improved SearchForm test suite #688
Conversation
Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com>
Codecov Report
@@ Coverage Diff @@
## master #688 +/- ##
==========================================
+ Coverage 94.36% 94.39% +0.03%
==========================================
Files 230 230
Lines 5959 5959
Branches 1448 1448
==========================================
+ Hits 5623 5625 +2
+ Misses 302 300 -2
Partials 34 34
Continue to review full report at Codecov.
|
wrapper = shallow(<SearchForm {...defaultProps} selectedService="svcB" />); | ||
const field = wrapper.find(`Field[name="operation"]`); | ||
expect(field).toMatchSnapshot(); | ||
}); |
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 I understand this correctly, these new tests are merely checking a single property on the field
. Wouldn't it be simpler to test that single property directly, like expect(field.foo).equalTo('bar')
, than to compare with a whole snapshot? The intent would be a lot more obvious and maintenance easier.
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.
Hi, I reworked those tests to test the single property instead of using the snapshots. Please let me know what you think, thanks!
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!
What about this specific test? Is it aiming to check only the boolean condition, or also the list of operations that are included in the dropdown?
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.
Oops, meant to remove that snapshot test in the last commit, I'm new to open source contributions so my bad!
Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com>
Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com>
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!
* improved searchform test suite Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> * improved searchform test suite, removede snapshots Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> * removed snapshot tests Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
* improved searchform test suite Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> * improved searchform test suite, removede snapshots Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> * removed snapshot tests Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
* improved searchform test suite Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> * improved searchform test suite, removede snapshots Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> * removed snapshot tests Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
* improved searchform test suite Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> * improved searchform test suite, removede snapshots Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> * removed snapshot tests Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
* improved searchform test suite Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> * improved searchform test suite, removede snapshots Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> * removed snapshot tests Signed-off-by: Amanda Chesin <Amanda.Chesin1@aexp.com> Signed-off-by: vvvprabhakar <vvvprabhakar@gmail.com>
Signed-off-by: Amanda Chesin Amanda.Chesin1@aexp.com
Which problem is this PR solving?
Short description of the changes