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

Behat tests for orders in the Back Office. Various Behat features created and scenarios added #16525

Conversation

@tdavidsonas88
Copy link
Contributor

tdavidsonas88 commented Nov 25, 2019

Questions Answers
Branch? develop
Description? Behat tests for orders in the Back Office
Type? improvement
Category? TE
BC breaks? no
Deprecations? no
Fixed ticket? #14480
How to test? QA testing not needed. Travis build should be green

This change is Reviewable

Handlers coverage:
CreateEmptyCustomerHandler
UpdateCartAddressesHandler
UpdateProductQuantityInCartHandler
AddOrderFromBackOfficeHandler
UpdateOrderStatusHandler
GetOrderForViewingHandler
UpdateOrderShippingDetailsHandler
AddPaymentHandler
DuplicateOrderCartHandler
GetCartInformationHandler
SearchProductsHandler
AddProductToOrderHandler
GenerateInvoiceHandler
SetFreeShippingToCartHandler
BulkChangeOrderStatusHandler
ChangeOrderDeliveryAddressHandler

@tdavidsonas88 tdavidsonas88 requested a review from PrestaShop/prestashop-core-developers as a code owner Nov 25, 2019
@prestonBot

This comment has been minimized.

Copy link
Collaborator

prestonBot commented Nov 25, 2019

Hello @tdavidsonas88!

This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community!

@tdavidsonas88 tdavidsonas88 changed the title Behat tests for changing orders status WIP: Behat tests for changing orders status Nov 25, 2019
@tdavidsonas88

This comment has been minimized.

Copy link
Contributor Author

tdavidsonas88 commented Nov 26, 2019

Travis fails because of the reason not related this pull request scope:

Tests\Integration\Behaviour\Features\Context\CLDRFeatureContext::assertDisplayPrice()
Displayed price is "14 789,54 " but "14 789,54 €" was expected (RuntimeException)
And database contains 1 rows of currency "EUR"

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Nov 26, 2019

  Background:                                                # Features/Scenario/Order/update_order_shipping_details.feature:8
    Given the current currency is "USD"                      # Tests\Integration\Behaviour\Features\Context\Domain\CartFeatureContext::addCurrencyToContext()
    Given there is existing order with reference "XKBKNABJK" # Tests\Integration\Behaviour\Features\Context\Domain\OrderFeatureContext::thereIsExistingOrderWithReference()
  Scenario: Update order shipping details                                                     # Features/Scenario/Order/update_order_shipping_details.feature:12
sh: 1: -t: not found
    When I update order "XKBKNABJK" Tracking number to "TEST1234" and Carrier to "My carrier" # Tests\Integration\Behaviour\Features\Context\Domain\OrderFeatureContext::iUpdateOrderTrackingNumberToAndCarrierTo()
      An error occurred while sending an email to the customer. (PrestaShop\PrestaShop\Core\Domain\Order\Exception\TransistEmailSendingException)
    Then order "XKBKNABJK" has Tracking number "TEST1234"                                     # Tests\Integration\Behaviour\Features\Context\Domain\OrderFeatureContext::orderHasTrackingNumber()
    And order "XKBKNABJK" has Carrier "My carrier"                                            # Tests\Integration\Behaviour\Features\Context\Domain\OrderFeatureContext::orderHasCarrier()
    When I update order "XKBKNABJK" Tracking number to "TEST123" and Carrier to "0"           # Tests\Integration\Behaviour\Features\Context\Domain\OrderFeatureContext::iUpdateOrderTrackingNumberToAndCarrierTo()
    Then order "XKBKNABJK" has Tracking number "TEST123"                                      # Tests\Integration\Behaviour\Features\Context\Domain\OrderFeatureContext::orderHasTrackingNumber()
    And order "XKBKNABJK" has Carrier "0"     

I think we also need to disable email sending for tests. I remember seeing something to do that in app/config/config_test.yml (see swiftmailer config), it might need to be reused or config is false

@matks

This comment has been minimized.

Copy link
Contributor

matks commented Nov 26, 2019

Travis fails because of the reason not related this pull request scope:

Tests\Integration\Behaviour\Features\Context\CLDRFeatureContext::assertDisplayPrice()
Displayed price is "14 789,54 " but "14 789,54 €" was expected (RuntimeException)
And database contains 1 rows of currency "EUR"

Well it does look like a bug 🤔 as indeed the symbol is missing. @matthieu-rolland can you tell us if this looks like one of these CLDR bugs you fixed before ?

@tdavidsonas88

This comment has been minimized.

Copy link
Contributor Author

tdavidsonas88 commented Nov 27, 2019

I think we also need to disable email sending for tests. I remember seeing something to do that in app/config/config_test.yml (see swiftmailer config), it might need to be reused or config is false

@matks @rokaszygmantas in app/config/config_test.yml swiftmailer settings are already correct:

swiftmailer:
disable_delivery: true

Since legacy code is sending email using @mail::Send in OrderCarrier.php, the temporary solution while code is being migrated would be changing PS_MAIL_METHOD to 3 in the testdb using:

Background:
Given email sending is disabled

Fixed it like this with the last commit to this branch.

@tdavidsonas88 tdavidsonas88 force-pushed the tdavidsonas88:feature/change-order-status=integration-tests branch from 1ec4b3d to 06b3b0f Nov 27, 2019
@tdavidsonas88 tdavidsonas88 changed the title WIP: Behat tests for changing orders status WIP: Behat tests for orders in the Back Office Nov 28, 2019
@tdavidsonas88 tdavidsonas88 force-pushed the tdavidsonas88:feature/change-order-status=integration-tests branch from 373f332 to 234506c Nov 28, 2019
@tdavidsonas88

This comment has been minimized.

Copy link
Contributor Author

tdavidsonas88 commented Nov 28, 2019

Travis fails because of the reason not related this pull request scope:
Tests\Integration\Behaviour\Features\Context\CLDRFeatureContext::assertDisplayPrice()
Displayed price is "14 789,54 " but "14 789,54 €" was expected (RuntimeException)
And database contains 1 rows of currency "EUR"

Well it does look like a bug as indeed the symbol is missing. @matthieu-rolland can you tell us if this looks like one of these CLDR bugs you fixed before ?

@rokaszygmantas @matks @matthieu-rolland actually this one is related to the test double I've created for the AdminController in the OrderFeatureContext -> @BeforeScenario -> $adminControllerTestDouble->php_self.

php_self property was missing in this case. So this failure of test is fixed.

Now the two tests are failing because of the issue created today: #16582

When the issue is fixed - the tests should pass;

Or If the business logic of the failing Behat tests is wrong, please let me know and I'll fix the scenarios.

@matthieu-rolland

This comment has been minimized.

Copy link
Contributor

matthieu-rolland commented Nov 28, 2019

Travis fails because of the reason not related this pull request scope:
Tests\Integration\Behaviour\Features\Context\CLDRFeatureContext::assertDisplayPrice()
Displayed price is "14 789,54 " but "14 789,54 €" was expected (RuntimeException)
And database contains 1 rows of currency "EUR"

Well it does look like a bug 🤔 as indeed the symbol is missing. @matthieu-rolland can you tell us if this looks like one of these CLDR bugs you fixed before ?

@matks Not really, the CLDR issues in tests were related to calculation results, not to formatting. 🤔

@tdavidsonas88 tdavidsonas88 changed the title WIP: Behat tests for orders in the Back Office Behat tests for orders in the Back Office. update_order_status, update_order_shipping_details, add_payment Behat features created Nov 29, 2019
@tdavidsonas88 tdavidsonas88 changed the title Behat tests for orders in the Back Office. update_order_status, update_order_shipping_details, add_payment Behat features created WIP: Behat tests for orders in the Back Office. Various Behat features created and scenarios added Nov 29, 2019
@tdavidsonas88 tdavidsonas88 changed the title WIP: Behat tests for orders in the Back Office. Various Behat features created and scenarios added Behat tests for orders in the Back Office. Various Behat features created and scenarios added Dec 3, 2019
@tdavidsonas88 tdavidsonas88 changed the title Behat tests for orders in the Back Office. Various Behat features created and scenarios added WIP: Behat tests for orders in the Back Office. Various Behat features created and scenarios added Dec 3, 2019
@tdavidsonas88 tdavidsonas88 force-pushed the tdavidsonas88:feature/change-order-status=integration-tests branch from 2ea5cb7 to f43eee9 Dec 3, 2019
@tdavidsonas88 tdavidsonas88 changed the title WIP: Behat tests for orders in the Back Office. Various Behat features created and scenarios added Behat tests for orders in the Back Office. Various Behat features created and scenarios added Dec 3, 2019
@tdavidsonas88 tdavidsonas88 force-pushed the tdavidsonas88:feature/change-order-status=integration-tests branch from db0bcc5 to a909fe0 Dec 5, 2019
@tdavidsonas88 tdavidsonas88 changed the title Behat tests for orders in the Back Office. Various Behat features created and scenarios added WIP: Behat tests for orders in the Back Office. Various Behat features created and scenarios added Dec 5, 2019
@Progi1984 Progi1984 added the WIP label Dec 6, 2019
@tdavidsonas88 tdavidsonas88 force-pushed the tdavidsonas88:feature/change-order-status=integration-tests branch from a909fe0 to 0b1b8f5 Dec 6, 2019
@tdavidsonas88 tdavidsonas88 changed the title WIP: Behat tests for orders in the Back Office. Various Behat features created and scenarios added Behat tests for orders in the Back Office. Various Behat features created and scenarios added Dec 9, 2019
@matks matks added the migration label Dec 12, 2019
@tdavidsonas88 tdavidsonas88 force-pushed the tdavidsonas88:feature/change-order-status=integration-tests branch from 8e3c69e to d14d0a7 Jan 2, 2020
@matks matks removed the WIP label Jan 2, 2020
@matks
matks approved these changes Jan 2, 2020
@matks matks merged commit 2e12091 into PrestaShop:develop Jan 2, 2020
2 checks passed
2 checks passed
PrettyCI Code formatting
Details
Travis CI - Pull Request Build Passed
Details
@matks

This comment has been minimized.

Copy link
Contributor

matks commented Jan 2, 2020

Thank you @tdavidsonas88

@matks matks added this to the 1.7.7.0 milestone Jan 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.