Skip to content
This repository has been archived by the owner on Dec 19, 2019. It is now read-only.

GraphQL-431: Test coverage: Using the "use_for_shipping" option with multishipping is not possible #452

Closed
wants to merge 6 commits into from
Closed

GraphQL-431: Test coverage: Using the "use_for_shipping" option with multishipping is not possible #452

wants to merge 6 commits into from

Conversation

AleksLi
Copy link
Contributor

@AleksLi AleksLi commented Mar 7, 2019 •
edited
Loading

Covered SetBillingAddressOnCartTest with multiple shipping addresses in a quote. Added Fixture for that.

Description (*)

Fixed Issues (if relevant)

  1. Test coverage: Using the "use_for_shipping" option with multishipping is not possible. #431: Test coverage: Using the "use_for_shipping" option with multishipping is not possible

Manual testing scenarios (*)

  1. Run the tests of SetBillingAddressOnCartTest for all the customer types Guest|Saved Customer.

naydav
naydav suggested changes Mar 8, 2019
View reviewed changes
dev/tests/integration/testsuite/Magento/Checkout/_files/quote_with_multiple_addresses_saved.php Outdated
@@ -0,0 +1,26 @@
<?php
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already the same fixture
Please, check the same class, it should be used in another testcases

BTW if we create fixture we MUST create rollback
Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@naydav I haven't found it. Can you take a look and give me the name. I did look at the whole project.

...ests/api-functional/testsuite/Magento/GraphQl/Quote/Customer/SetBillingAddressOnCartTest.php Outdated
@@ -403,6 +403,51 @@ public function testSetBillingAddressIfCustomerIsNotOwnerOfAddress()
$this->graphQlQuery($query, [], '', $this->getHeaderMap('customer2@search.example.com'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to add the same case for Guest
https://github.com/magento/graphql-ce/blob/3469f4b251cdc4874048dfff0374f818cb757964/dev/tests/api-functional/testsuite/Magento/GraphQl/Quote/Guest/SetBillingAddressOnCartTest.php

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@naydav Yes, I will do that

@AleksLi AleksLi requested a review from naydav March 9, 2019 10:51
@AleksLi AleksLi force-pushed the 431-test-coverate-qoute-multiple-addresses branch from 2b57cf0 to eb82542 Compare March 9, 2019 12:44
naydav
naydav suggested changes Mar 12, 2019
View reviewed changes
.../integration/testsuite/Magento/Checkout/_files/guest_quote_with_multiple_addresses_saved.php Outdated
@@ -0,0 +1,61 @@
<?php
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, reuse current fixtures files
You can find examples for all of the cases in

https://github.com/magento/graphql-ce/tree/2.3-develop/dev/tests/api-functional/testsuite/Magento/GraphQl/Quote/Customer

https://github.com/magento/graphql-ce/tree/2.3-develop/dev/tests/api-functional/testsuite/Magento/GraphQl/Quote/Guest
(also in the same testcase class)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@naydav ok. I'll do that. Thanks.

@naydav
Copy link
Contributor

naydav commented Mar 13, 2019

@AleksLi
Implementation is overhead. You should not use create or modify quote or address model

What need to do:

  1. Use the quote with addresses fixture as a precondition (could be found in the same file)
  2. During test try to assign billing address with use_for_shipping=true flag
  3. Check error message

Thanks

@AleksLi AleksLi force-pushed the 431-test-coverate-qoute-multiple-addresses branch from 9251ec4 to f65c6c0 Compare March 19, 2019 19:01
@AleksLi AleksLi requested a review from naydav March 23, 2019 19:54
@AleksLi
Copy link
Contributor Author

AleksLi commented Mar 23, 2019

@naydav please take a look on this one again. I know that some of the checks are FAILED but I don't know why, actually. I've seen the reason and it looks weird as for me.

@AleksLi AleksLi force-pushed the 431-test-coverate-qoute-multiple-addresses branch from f65c6c0 to c8c9c4d Compare March 28, 2019 19:48
@naydav
Copy link
Contributor

naydav commented Apr 2, 2019

Was covered in the scope of
#321
#330

@naydav naydav closed this Apr 2, 2019
@ghost
Copy link

ghost commented Apr 2, 2019

Hi @AleksLi, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

@okorshenko okorshenko deleted the 431-test-coverate-qoute-multiple-addresses branch December 18, 2019 22:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@naydav naydav Awaiting requested review from naydav

Assignees
No one assigned
Labels
Partner: Vaimo partners-contribution Progress: needs update
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AleksLi @naydav @magento-engcom-team