[Behat] Decouple cart setup from interaction – Given I added vs When I add#17785
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
4a35a6b to
724a723
Compare
❌ Preview Environment deleted from BunnyshellAvailable commands:
|
1dd96d6 to
430476e
Compare
416c871 to
ac60b69
Compare
ac60b69 to
4feaa16
Compare
4feaa16 to
a4e5f66
Compare
d042431 to
03a556a
Compare
03a556a to
b3d1308
Compare
GSadee
left a comment
There was a problem hiding this comment.
I believe that my comments could be fixed in the next iteration
| And I proceed with "Free" shipping method and "Offline" payment | ||
| And I added product "PHP T-Shirt" to the cart | ||
| And I addressed the cart | ||
| When I proceeded with "Free" shipping method and "Offline" payment method |
There was a problem hiding this comment.
| When I proceeded with "Free" shipping method and "Offline" payment method | |
| When I proceed with "Free" shipping method and "Offline" payment method |
| @@ -15,7 +15,7 @@ Feature: Removing an address from my book | |||
|
|
|||
| @api @ui @javascript | |||
There was a problem hiding this comment.
Could we not remove this js tag?
| @@ -15,7 +15,7 @@ Feature: Viewing addresses created after checkout | |||
|
|
|||
| @api @ui @javascript | |||
| Scenario: Changing the currency of my cart | ||
| Given I have product "The Pug Mug" in the cart | ||
| When I switch to the "GBP" currency | ||
| When I add product "The Pug Mug" to the cart |
There was a problem hiding this comment.
This one could be Given and perhaps we wouldn't need the js tag 🤔
| Scenario: Increasing quantity of an item in cart | ||
| Given I see the summary of my cart | ||
| When I change product "T-Shirt banana" quantity to 2 in my cart | ||
| Given I added product "T-Shirt banana" to the cart |
There was a problem hiding this comment.
This given step could stay in Background as it is the same for both scenarios here
There was a problem hiding this comment.
Adding to that we could also move the summary part to the background as well, just rephrase it to smth like I am viewing the summary of my cart
| And I am a logged in customer | ||
|
|
||
| @api @ui @javascript | ||
| @api @ui @mink:chromedriver |
| @api @ui @mink:chromedriver | ||
| Scenario: Preventing customer from completing checkout with no longer available payment method | ||
| Given I have product "PHP T-Shirt" in the cart | ||
| When I add product "PHP T-Shirt" to the cart |
| @@ -13,10 +13,10 @@ Feature: Registering a new account after checkout | |||
| @no-api @ui @javascript | |||
| @api @ui @javascript | ||
| Scenario: Seeing the same shipping and billing address on order summary | ||
| Given I have product "Lannister Coat" in the cart | ||
| When I add product "Lannister Coat" to the cart |
| @@ -21,8 +21,8 @@ Feature: Seeing order locale on order summary page | |||
|
|
|||
| @no-api @ui @javascript | |||
| Given I changed my current channel to "United States" | ||
| And I have product "Angel T-Shirt" in the cart | ||
| And I specified the billing address as "Los Angeles", "Frost Alley", "90210", "United States" for "Lucifer Morningstar" | ||
| And I proceed with "Free" shipping method and "Offline" payment | ||
| And I added product "Angel T-Shirt" to the cart | ||
| And I addressed the cart | ||
| When I proceed with "Free" shipping method and "Offline" payment | ||
| And I confirm my order | ||
| And I changed my current channel to "Great Britain" | ||
| And I had product "Angel T-Shirt" in the cart | ||
| And I have product "Angel T-Shirt" in the cart | ||
| And I specified the billing address as "Los Angeles", "Frost Alley", "90210", "United States" for "Lucifer Morningstar" | ||
| And I proceed with "Free" shipping method and "Offline" payment | ||
| When I confirm my order |
There was a problem hiding this comment.
As it is, we have doubled Whens.
Everything up to I change my current channel could be done in the setup.
| When I add product "PHP T-Shirt" to the cart | ||
| And I am at the checkout addressing step | ||
| And my billing address is fulfilled automatically through default address | ||
| When I specify the first and last name as "Mike Ross" for billing address |
There was a problem hiding this comment.
Doubled When;
Quite frankly, we could setup everything up to I confirm my order or better yet I complete the checkout or something like that.
| And I added product "T-Shirt banana" to the cart | ||
| When I add this product to the cart |
There was a problem hiding this comment.
| And I added product "T-Shirt banana" to the cart | |
| When I add this product to the cart | |
| And I have product "T-Shirt banana" in the cart | |
| When I add this product to the cart again |
| Scenario: Increasing quantity of an item in cart | ||
| Given I see the summary of my cart | ||
| When I change product "T-Shirt banana" quantity to 2 in my cart | ||
| Given I added product "T-Shirt banana" to the cart |
There was a problem hiding this comment.
Adding to that we could also move the summary part to the background as well, just rephrase it to smth like I am viewing the summary of my cart
| When I add product "PHP T-Shirt" to the cart | ||
| And I go to the checkout addressing step | ||
| When I specify the email as "francis@underwood.com" |
There was a problem hiding this comment.
Doubled Whens; many such cases(tm) in this file.
| * @Given /^I added (\d+) of (them) to (?:the|my) (cart)$/ | ||
| */ | ||
| public function iHaveAddedProductsToTheCart(int $quantity, ProductInterface $product, ?string $tokenValue): void | ||
| public function iAddedGivenQuantityOfProductsToTheCart(int $quantity, ProductInterface $product, ?string $tokenValue): void |
There was a problem hiding this comment.
Why, the original template is still in place, there's no need for this change
| } | ||
|
|
||
| /** Set the token in the shared storage to have access both in UI and API to resources created in the setup context */ | ||
| private function storeUserToken(UserInterface $user): void |
There was a problem hiding this comment.
| private function storeUserToken(UserInterface $user): void | |
| private function saveUserToken(UserInterface $user): void |
🤷
Although, could we add the user saving in here as well? Would be a bit cleaner
|
|
||
| $this->securityService->logInWithRememberMe($user); | ||
|
|
||
| $this->storeUserToken($user); |
There was a problem hiding this comment.
Don't think this one is necessary;
Can you even use remember me through the API?
| { | ||
| $carts = $this->orderRepository->findBy( | ||
| ['state' => OrderInterface::STATE_CART], | ||
| ['state' => OrderCheckoutStates::STATE_CART], |
There was a problem hiding this comment.
To be reverted.
Although the value is the same, the field we're using is state, not checkout_state
| { | ||
| if (!isset($this->clipboard[$key])) { | ||
| throw new \InvalidArgumentException(sprintf('There is no current resource for "%s"!', $key)); | ||
| throw new SharedStorageElementNotFoundException($key); |
There was a problem hiding this comment.
👌
Similar should be made for ::getLatestResource or just 2 factory methods within the exception 🤷
…en I add` - Promotion & Some other scenarios (#17818) | Q | A |-----------------|----- | Branch? | 2.0 <!-- see the comment below --> | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no <!-- don't forget to update the UPGRADE-*.md file --> | License | MIT Continuation of: - #17785 - #17782 - #17783 Until now, the checkout-related scenarios have been set up as follows: • UI – via web actions (e.g. `Given I have a product ...` actually triggered browser actions to prepare the initial step), • API – a mix of messenger commands and API calls. During the development of Sylius 2.0, with the introduction of dynamic components, all scenarios involving product-related actions had to be marked as JS to work correctly. This significantly increased build time and reduced overall stability. This PR focuses on decoupling the Behat architecture to unify the test setup for both UI and API. The goal is to eliminate most JS tags, which should lead to greater stability. Some performance improvements are already noticeable. This PR doesn’t yet include all refactored scenarios, but at this stage we already observe the following performance gains: • For non-JS scenarios, the timing is likely similar — some JS scenarios have been converted to non-JS, but at the same time, we benefit from faster setup thanks to the refactor (using messenger commands instead of API calls or UI interactions to prepare tests). • For JS scenarios executed via Chromedriver, the same applies — some Panther scenarios have been switched to Chromedriver, and performance remains comparable. JS with Panther: **~30 min → ~20 min**
…` and related scenarios) (#17816) | Q | A |-----------------|----- | Branch? | 2.0 <!-- see the comment below --> | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no <!-- don't forget to update the UPGRADE-*.md file --> | License | MIT Continuation of: - #17785 - #17782 - #17783 Until now, the checkout-related scenarios have been set up as follows: • UI – via web actions (e.g. `Given I have a product ...` actually triggered browser actions to prepare the initial step), • API – a mix of messenger commands and API calls. During the development of Sylius 2.0, with the introduction of dynamic components, all scenarios involving product-related actions had to be marked as JS to work correctly. This significantly increased build time and reduced overall stability. This PR focuses on decoupling the Behat architecture to unify the test setup for both UI and API. The goal is to eliminate most JS tags, which should lead to greater stability. Some performance improvements are already noticeable. This PR doesn’t yet include all refactored scenarios, but at this stage we already observe the following performance gains: • For non-JS scenarios, the timing is likely similar — some JS scenarios have been converted to non-JS, but at the same time, we benefit from faster setup thanks to the refactor (using messenger commands instead of API calls or UI interactions to prepare tests). • For JS scenarios executed via Chromedriver, the same applies — some Panther scenarios have been switched to Chromedriver, and performance remains comparable. JS with Panther: **~30 min → ~20 min**
Continuation of:
SaveContextfor more flexible suite configuration management #17783Until now, the checkout-related scenarios have been set up as follows:
• UI – via web actions (e.g.
Given I have a product ...actually triggered browser actions to prepare the initial step),• API – a mix of messenger commands and API calls.
During the development of Sylius 2.0, with the introduction of dynamic components, all scenarios involving product-related actions had to be marked as JS to work correctly. This significantly increased build time and reduced overall stability.
This PR focuses on decoupling the Behat architecture to unify the test setup for both UI and API. The goal is to eliminate most JS tags, which should lead to greater stability. Some performance improvements are already noticeable.
This PR doesn’t yet include all refactored scenarios, but at this stage we already observe the following performance gains:
• For non-JS scenarios, the timing is likely similar — some JS scenarios have been converted to non-JS, but at the same time, we benefit from faster setup thanks to the refactor (using messenger commands instead of API calls or UI interactions to prepare tests).
• For JS scenarios executed via Chromedriver, the same applies — some Panther scenarios have been switched to Chromedriver, and performance remains comparable.
JS with Panther: ~30 min → ~20 min