-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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] [E2E] Delete shipping rate from its details page #4625
Conversation
🦋 Changeset detectedLatest commit: 3f2bca7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
d9e3db9
to
e2ff683
Compare
e2ff683
to
7a2a545
Compare
7a2a545
to
2e6e33c
Compare
2e6e33c
to
5aa0cb7
Compare
async clickDeleteShippingMethod() { | ||
await this.priceBasedRatesSection.locator(this.deleteShippingMethodButton).click(); | ||
async clickDeleteShippingRateFromTheList() { | ||
await this.priceBasedRatesSection.locator(this.deleteShippingRateButtonOnList).click(); |
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.
Is this not gonna be confusing when deleting weight base shipping?
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.
Would clickDeletePriceBasedShippingMethod be more obvious and descriptive?
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.
const weightBasedRate = SHIPPING_METHODS.shippingMethodWithRatesToBeDeleted.rates.weightBasedRateToBeDeleted.name; | ||
|
||
await shippingMethodsPage.gotoExistingShippingRate(shippingMethodId, shippingRateId); | ||
await expect(shippingMethodsPage.basePage.pageHeader).toBeVisible(); |
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.
Since you await input to be visible inside gotoExistingShippingRate then this expect has no use, right?
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.
await shippingMethodsPage.gotoExistingShippingMethod( | ||
SHIPPING_METHODS.shippingMethodWithRatesToBeDeleted.id, | ||
); | ||
await expect(shippingMethodsPage.weightBasedRatesSection).toContainText("No shipping rates found"); | ||
await expect(shippingMethodsPage.weightBasedRatesSection).not.toContainText(weightBasedRate); |
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 do not check integration between views so I would skip navigating to other views to confirm the action result. Check all you need on the screen that was loaded after deletion, do not go to other views to double-check results. Otherwise, this test would not have an end, there are so many views to be checked
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.
page, | ||
}) => { | ||
const shippingMethodsPage = new ShippingMethodsPage(page); | ||
const shippingMethodId = SHIPPING_METHODS.shippingMethodWithRatesToBeDeleted.id; |
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.
Just to clarify, this ID is actually the Shipping Zone ID and not the Shipping Method ID.
It covers TC SALEOR_35
What type of PR is this?
Related Issues or Documents
Usage Instructions, Screenshots, Recordings
Have you written tests?
[Optional] Description