search

Automattic / woocommerce-payments

Accept payments via credit card. Manage transactions within WordPress.
https://wordpress.org/plugins/woocommerce-payments/
Other
173 stars 69 forks source link

Write new e2e test: Merchant / Onboarding / Manual plugin installation and setup #2201

Open veljkho opened 3 years ago

veljkho commented 3 years ago

Write a new e2e test that covers the following flow:

veljkho commented 3 years ago

Hi @vbelolapotkov Is this a valid flow to automate since it requires manually installing of the plugin? Here's the link of the flow description. Setup onboarding will be covered in this pull request.

vbelolapotkov commented 3 years ago

Hey @veljkho ! Thanks for the ping, here! I'd say it's still valid test. In mentioned PR the setup is being triggered from WC OBW or WC Task list, while in this scenario plugin setup is triggered from connect page (default page when user navigates to Payments menu).

To make it even more useful, we might include a flow when store's country (in WC settings) not in the list of supported countries by WC Pay. That should trigger a modal window before starting KYC flow.

As for plugin installation part, manual installation assumes the plugin being added to the site manually, rather than via OBW or setup task flow, I think we can omit that step and test only setup part from connect page.

veljkho commented 3 years ago

Hey @vbelolapotkov What "Connect" button you were referring to? Does it require a connection with WordPress or WooCommerce account? I know about this page as the first page when accessing newly installed WCP and also this page. If the flow requires a connection to a real account, I think it will be another blocked flow.

vbelolapotkov commented 3 years ago

Hey @veljkho

What "Connect" button you were referring to?

I was referring to connect page which has setup button. I think it's labeled "Finish setup" now.

Does it require a connection with WordPress or WooCommerce account?

It triggers connection setup with WP, so we'll probably need to fake it. No need for WC account.

I know about this page as the first page when accessing newly installed WCP

Yep, that's what we call Connect page.

and also this page

This is an account overview page. It's different and displayed after account is already connected.

If the flow requires a connection to a real account, I think it will be another blocked flow.

What do you mean by "real account"? All other tests are run with real stripe account connected, we fake JP connection but e2e server is configured with real stripe account available in Stripe Dashboard :) So we could be able to programmatically alter server behavior within e2e environment to mock connections, etc. AFAIK webhooks are not involved into onboarding process, only redirects.

veljkho commented 3 years ago

Hey @vbelolapotkov Did you mean about this page? This is the setup after clicking Finish Setup button... and it's already created/prefilled/preconfigured during E2E setup. It doesn't look possible to test the setup flow of this since it has already been preconfigured in test mode. My screenshots are from a fresh new site and WCP installed, so the Connect page is present there. But, in the E2E env. it is not even possible to access this Setup page.

vbelolapotkov commented 3 years ago

I was referring this page @veljkho

image

Technically we should be able to mock Stripe's KYC flow (everything what's happening on Stripe end) in E2E tests.

veljkho commented 3 years ago

Yes, I was also referring to that page. When you click on Finish Setup blue button, you'll be redirected to this page. Since we already have connected account within the environment (probably fake connection, test mode), we don't have the possibility to see the page you have showed, because already connected account. If we mock or similar, don't see the purpose of automating that kind of flow since it's not what the user really does with his real account, it wouldn't be a real connection or test case. But, since we already have the account connected, it could be one of the valid tests that ran when creating the environment. What do you think? @vbelolapotkov

vbelolapotkov commented 3 years ago

When you click on Finish Setup blue button, you'll be redirected to this page. Since we already have connected account within the environment (probably fake connection, test mode), we don't have the possibility to see the page you have showed, because already connected account.

Yep, that's right, but we can spin up as many environments as we need :) Or skip update setup script to remove connection to existing account and run onboarding tests first, before running all other tests.

If we mock or similar, don't see the purpose of automating that kind of flow since it's not what the user really does with his real account, it wouldn't be a real connection or test case.

I can't fully agree here. We'd mock only Stripe screens to bypass them, but E2E test would check WC Pay logic (e.g. modal for unsupported country), and integration between WC Pay and WP/WC Core (WC Admin) which might be easily broken by new WP/WC release.

veljkho commented 3 years ago

We'd mock only Stripe screens to bypass them, but E2E test would check WC Pay logic (e.g. modal for unsupported country), and integration between WC Pay and WP/WC Core (WC Admin) which might be easily broken by new WP/WC release.

@vbelolapotkov I am not sure about mocking Stripe screens, as this would be an E2E test and the best would be to leave it for manual testing as the goal of an E2E test overall is to make sure all those connections work. I get mocking for a unit test more.

vbelolapotkov commented 3 years ago

I am not sure about mocking Stripe screens, as this would be an E2E test and the best would be to leave it for manual testing

It's hard/impossible to test manually with multiple versions of WC. That's why automated test would help us to reveal if we break compatibility with older version of WC.

as the goal of an E2E test overall is to make sure all those connections work. I get mocking for a unit test more.

I agree, but the whole point is we can't run such unit test with specific version of WC. As I said the purpose of such automation is to test integration between WC and WC Pay, excluding connection with Stripe and Jetpack. However, if you have better ideas of how we could test this, I'm more than happy to consider alternatives.

veljkho commented 3 years ago

Can I move this flow to blocked flows for this current project thread so I can close it, then I'll make a notice that this flow will be moved to the nest project thread, so this flow is not blocking this current project and we don't have currently the best idea how to approach this flow, there must be a better solution than what we wanted. @vbelolapotkov

vbelolapotkov commented 3 years ago

Can I move this flow to blocked flows for this current project thread so I can close it

@veljkho , sure, go ahead please and thanks for the discussion 🙂

htdat commented 1 year ago

This issue impacts CI/CD Pipeline, so assigning to Harmony (based on team responsibilities Pc2DNy-3z-p2) cc @deepakpathania as team lead. Assigning as part of Gamma Triage process PcreKM-yM-p2.

deepakpathania commented 1 year ago

@htdat Writing new E2E tests isn't in the scope for Harmony and as such should be directed to the relevant team owning the product area the test is trying to cover. You can refer to this P2 for reference. paJDYF-7q7-p2