-
Notifications
You must be signed in to change notification settings - Fork 179
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(labware-creator): more test refactoring #7853
Conversation
when(displayAsTubeMock) | ||
.calledWith(formikConfig.initialValues) |
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.
@mcous and i had a long chat about expect
vs expectCalledWith
(i.e. mocks vs stubs). i documented the outcome that we prefer mocks over stubs in the frontend cookbook decision log
i dont think we need to be too pedantic about this, just thought i'd comment. what do u think?
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.
I'm kinda confused by the discussion and how it relates to that conclusion. The decision log says mocks would do expectCalledWith
, but @mcous is arguing against expectCalledWith
and it seems like you meet him there, so I don't get how you two concluded mocks over stubs it seems like you both said the opposite?
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.
ARGH I am so sorry @IanLondon I have been using the terms backwards, which is super confusing, 10000% my bad.
The decision is to prefer STUBS (pre canned answers, making no assertions when called in an unexpected way). Super sorry for using the wrong term, must have been really frustrating trying to understand what we meant :(
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.
I fixed the decision log miswording
Codecov Report
@@ Coverage Diff @@
## edge #7853 +/- ##
=======================================
Coverage ? 83.22%
=======================================
Files ? 355
Lines ? 22267
Branches ? 0
=======================================
Hits ? 18531
Misses ? 3736
Partials ? 0 Continue to review full report at Codecov.
|
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.
🧸 LGTM - also doesnt look like it will interfere with @smb2268's Total Height PR
Overview
This PR updates existing tests for the different sections in LC.
Changelog
Review requests
Risk assessment
Low, test refactors in LC only