Skip to content
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

Increase test coverage for packages/peregrine/lib/talons/CheckoutPage #3024

Merged
merged 26 commits into from
Mar 3, 2021

Conversation

jcalcaben
Copy link
Contributor

@jcalcaben jcalcaben commented Feb 25, 2021

Description

Increase coverage for files under packages/peregrine/lib/talons/CheckoutPage

NOTE:
The uncovered lines in the report represent branching logic paths that appear to be unreachable.

Line 12 in CheckoutPage/AddressBook/useAddressCard.js: Address cannot be undefined because the destructuring in Line 5 would cause an error before it could reach Line 12.

Line 132 in CheckoutPage/PaymentInformation/useCreditCard.js: Cannot test the ( false || true ) path for this line since the only time dropInLoading is set to false, the stepNumber is always set to 0.

Related Issue

Closes PWA-996

Acceptance

any developer

Verification Stakeholders

any developer

Specification

Verification Steps

  1. Run tests for this specific directory: yarn test --collectCoverageFrom="packages/peregrine/lib/talons/CheckoutPage/**/*.js" packages/peregrine/lib/talons/CheckoutPage/
  2. Verify acceptable test coverage

Screenshots / Screen Captures (if appropriate)

Checklist

  • I have added tests to cover my changes, if necessary.
  • I have added translations for new strings, if necessary.
  • I have updated the documentation accordingly, if necessary.

@jcalcaben jcalcaben added the version: Patch This changeset includes backwards compatible bug fixes. label Feb 25, 2021
@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented Feb 25, 2021

Messages
📖

Associated JIRA tickets: PWA-996.

📖 DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).
📖

Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.

Generated by 🚫 dangerJS against d2b7cfb

@sirugh
Copy link
Contributor

sirugh commented Feb 25, 2021

I'm seeing some test failures when running yarn test:dev but also the command you gave in your verification steps. Can you double check these please :)

@sirugh
Copy link
Contributor

sirugh commented Feb 25, 2021

The untestable places seem like refactor targets if they're easy enough to fix up and the logic is identical. What do you thnink?

@larsroettig larsroettig self-requested a review February 28, 2021 10:46
larsroettig
larsroettig previously approved these changes Feb 28, 2021
sirugh
sirugh previously approved these changes Mar 1, 2021
Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to do a quick merge from dev to ensure the test coverage remains. Also had a thought about mocking queries that generally applies to any places you used that style in these tests. Regardless, I'll approve now and let you decide how to proceed.

Comment on lines 29 to 42
useMutation.mockImplementation(query => {
let result = [jest.fn(), { error: null }];

switch (query) {
case 'createAccountMutation':
result = [mockCreateAccount, { error: null }];
break;
case 'signInMutation':
result = [mockSignIn, { error: null }];
break;
}

return result;
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you try using the new query mocking approach, like here?

@jcalcaben jcalcaben dismissed stale reviews from sirugh and larsroettig via aaef8f1 March 1, 2021 16:19
@jcalcaben jcalcaben requested a review from sirugh March 1, 2021 16:20
Copy link
Contributor

@sirugh sirugh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bueno

@supernova-at supernova-at merged commit b414cd0 into develop Mar 3, 2021
@supernova-at supernova-at deleted the jimothy/pwa-996_CheckoutPage-test-coverage branch March 3, 2021 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:peregrine Progress: done version: Patch This changeset includes backwards compatible bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants