-
Notifications
You must be signed in to change notification settings - Fork 218
Playwright E2E tests: Multiple signed in roles #10561
Conversation
The release ZIP for this PR is accessible via:
Script Dependencies ReportThere is no changed script dependency between this branch and trunk. This comment was automatically generated by the TypeScript Errors Report
🎉 🎉 This PR does not introduce new TS errors. |
Size Change: 0 B Total Size: 1.61 MB ℹ️ View Unchanged
|
e15ac5b
to
9243cbd
Compare
9702ace
to
e827123
Compare
Hey @opr! My tests are failing, but I can't debug them locally. I can't run a single-file test using CLI or the VS Code extension (all 50 tests get executed in each instance). And for some reason, the tests are failing locally! I followed the suggestion from p1692138789517629-slack-C02UBB1EPEF, but without success! |
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 Saad, thanks for the PR. @gigitux and I discussed about auth a little, and I think we should use Multiple signed in roles to test for authenticated vs unauthenticated users.
Because of this I would recommend the following changes to this PR:
- Remove the
login
,logOut
, andisLoggedIn
utils from theFrontendUtils
class - Consider adding a
tests/auth.setup.ts
file as outlined in the playwright docs link above, log in as the user there and save the storage state. - In the test, create one context for each user type (logged in, logged out) and run the test for both users in the same test case.
Also, you may consider incorporating these tests in checkout-block-shipping.block_theme.side_effects.spec.ts
instead of a new file, adding the cases to the existing test there. (Side note, we may want to rename this file now since it's not really focusing on shipping anymore).
I have been thinking about it more and I think it is actually a better idea to test as much as we can in one test, rather than setting up new ones for each thing we want to check.
Let me know what you think!
I can't run a single-file test using CLI or the VS Code extension (all 50 tests get executed in each instance).
Go to playwright.side-effects.config.ts
and remove
dependencies: [ 'blockThemeConfiguration' ],
from the blockThemeWithGlobalSideEffects
config object.
Then the command I ran was:
npm run test:e2e:side-effects -- --headed tests/e2e/tests/checkout/checkout-block-place-order.block_theme.side_effects.spec.ts
@opr, I created the Error: Error reading storage state from playwright/.auth/customer.json:
ENOENT: no such file or directory, open 'playwright/.auth/customer.json' I believe the file
I believe the code will look like this: test( 'Guest and logged in users can place order', async ( {
pageObject,
frontendUtils,
page,
} ) => {
const customerContext = await browser.newContext({ storageState: 'playwright/.auth/customer.json' });
const customerPage = await adminContext.newPage();
await frontendUtils.emptyCart();
await frontendUtils.goToShop();
await frontendUtils.addToCart( SIMPLE_PHYSICAL_PRODUCT_NAME );
await frontendUtils.goToCheckout();
await expect(
await pageObject.selectAndVerifyShippingOption(
FREE_SHIPPING_NAME,
FREE_SHIPPING_PRICE
)
).toBe( true );
await pageObject.fillInCheckoutWithTestData();
await pageObject.placeOrder();
await expect(
customerPage.getByText( 'Your order has been received.' )
).toBeVisible();
const guestContext = await browser.newContext({ storageState: 'playwright/.auth/guest.json' });
const guestPage = await adminContext.newPage();
etc...
} ); I haven't tested it yet, but I was wondering how |
Hi @tarhi-saad where does this error appear? What steps can I take to reproduce it?
Apologies, I think I wrote something incorrectly in my review, I spoke more to Luigi and I think we should use one test per user-role. We should therefore write the test like this (from playwright docs) To ensure the {
name: 'authSetup',
testDir: '.',
testMatch: /auth.setup.ts/,
}, I then made the following projects depend on it: Example config:
Also, in Finally, the Hope this is useful 😎 |
273385d
to
01559bf
Compare
01559bf
to
9a47a6b
Compare
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.
Hey @tarhi-saad thanks for working on this, starting to look good, I think the auth steps are really valuable. I left some inline comments that should be addressed. Thanks!
tests/e2e/tests/checkout/checkout-block-place-order.block_theme.side_effects.spec.ts
Outdated
Show resolved
Hide resolved
@opr, I've incorporated all the feedback! However, the shipping test is failing. It seems that from the beginning, the authentication setup hasn't been functioning properly. We've been placing orders without authentication consistently (even when using |
// User roles file paths | ||
export const adminFile = '.auth/admin.json'; | ||
export const customerFile = '.auth/customer.json'; | ||
export const guestFile = '.auth/guest.json'; |
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.
We are saving these at the project root, but the .gitignore
only ignores things in e2e
. We should update these paths.
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.
@opr, when I run the tests locally, these files are created in tests/e2e/.auth/
, not on the project root.
Hi @tarhi-saad - during the auth setup, we were checking if we're logged out, it looks like the same context is used between setup steps, so the admin account is logged in. I reversed the logic, so now instead of checking if we're logged out, we will check if we're logged in (i.e., is the logout link visible) and we will log out before each test. |
eb315c3
to
622200d
Compare
622200d
to
1f2ccd2
Compare
We are putting this PR into draft for now! We will resume working on it when @gigitux is back! We need to figure out how our auth setup steps can be used by the work Luigi did to revert templates. Before proceeding, we need to understand that part fully (see Slack p1692889180189739/1692886577.310339-slack-C8X6Q7XQU). Slack bug discussion: p1692878370587469/1692714875.315809-slack-C8X6Q7XQU |
This PR has been marked as If deemed still relevant, the pr can be kept active by ensuring it's up to date with the main branch and removing the stale label. |
Co-authored-by: Thomas Roberts <[email protected]>
Co-authored-by: Thomas Roberts <[email protected]>
Co-authored-by: Thomas Roberts <[email protected]>
Co-authored-by: Thomas Roberts <[email protected]>
Co-authored-by: Thomas Roberts <[email protected]>
Co-authored-by: Thomas Roberts <[email protected]>
In this step of the Playwright's setup, we can add the multiple sign-in roles and keeping the admin logged by default. This fixes the issue of failing tests `logged out` error.
The admin profile is set by default
b750021
to
4fe2bb8
Compare
This PR moves the following tests to playwright and removes the puppeteer equivalent:
Setting up Playwright's multiple sign up roles
We are using Multiple signed in roles to test for authenticated vs. unauthenticated users. There are some required adjustments to fix #10561 (comment). We will update this section when the PR is ready to be merged!
Other Checks
[type]
label or a[skip-changelog]
label.Testing
Automated Tests
User Facing Testing
Ensure Playwright E2E tests pass and ensure Puppeteer E2Es also pass, so that we can be sure the removed tests don't impact on anything.
Test the multiple sign-in roles feature
tests/e2e/tests/checkout/checkout-block.block_theme.side_effects.spec.ts
at the lines39
and114
tests/e2e/tests/checkout/checkout-block.block_theme.side_effects.spec.ts
in debug modeMy Account
page in a new tab. Ensure the customerJane Smith
is logged inMy Account
page in a new tab. Ensure the user is logged outWooCommerce Visibility