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

PB Banner UI test #3178

Merged
merged 35 commits into from
Jun 1, 2021
Merged

PB Banner UI test #3178

merged 35 commits into from
Jun 1, 2021

Conversation

dpatil-magento
Copy link
Contributor

@dpatil-magento dpatil-magento commented May 16, 2021

Description

TODO: Describe your changes in detail here.

Related Issue

Closes https://jira.corp.magento.com/browse/PWA-1540.

Acceptance

Verification Stakeholders

Specification

Verification Steps

  1. Cypress banner.spec.js should pass
    Steps -
    Go to venia-integration-test dir
    Run yarn install
    Run yarn test
    Run banner.spec.js from cypress ui

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.

@PWAStudioBot
Copy link
Contributor

PWAStudioBot commented May 16, 2021

Fails
🚫 Missing information in PR. Please fill out the "Description" section.
Warnings
⚠️ Found the word "TODO" in the PR description. Just letting you know incase you forgot :)
Messages
📖

Associated JIRA tickets: PWA-1540.

📖 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.

If your PR is missing information, check against the original template here. At a minimum you must have the section headers from the template and provide some information in each section.

Generated by 🚫 dangerJS against 1c21330

@dpatil-magento dpatil-magento changed the title [DRAFT]PB Banner UI test PB Banner UI test May 17, 2021
failureThresholdType: 'percent'
});
});
});
Copy link
Contributor

@revanth0212 revanth0212 May 26, 2021

Choose a reason for hiding this comment

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

Suggested change
});
});

Just for consistency.

Copy link
Contributor

@revanth0212 revanth0212 left a comment

Choose a reason for hiding this comment

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

Tests look fine but some clean-up is needed on the assertions end.

@@ -0,0 +1,7 @@
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think we need this any more. We moved these to custom Cypress actions in the wishlist PR.

* @param {String} accountEmail accountEmail to createAccount
* @param {String} accountPassword accountPassword to createAccount
*/
export const createAccount = (
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont think we need these any more. We moved these to custom cypress actions. Or am I missing something?

.click();
};

export const assertCreateAccount = (
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move assertions to assertions folder.

* Utility function to assert empty wishlist
* @param {String} wishlistHeaderText wishlist page header text
*/
export const assertWishlistHeading = wishlistHeaderText => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move assertions to the assertions folder.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dont know how I ended up here but updated all, thanks. Please re-check :)

@sirugh sirugh added the version: Patch This changeset includes backwards compatible bug fixes. label May 28, 2021
@sirugh
Copy link
Contributor

sirugh commented May 28, 2021

Getting test failures for 1-3 - seems the snaps were saved at 1200 width but the test runner is now outputting 1440, at least on my machine.

Edit: Had to clear cypress chrome cache :/

@sirugh
Copy link
Contributor

sirugh commented Jun 1, 2021

Will merge after checks pass! Re-verified, had to clear local cypress chrome cache.

@sirugh sirugh merged commit e4dc477 into develop Jun 1, 2021
@sirugh sirugh deleted the dev/banner-cypress branch June 1, 2021 16:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Progress: done version: Patch This changeset includes backwards compatible bug fixes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants