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.

Sign up for GitHub

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

Jump to bottom

magento/magento2#: Unit test for \Magento\Bundle\Observer\AppendUpsellProductsObserver #26429

Merged
merged 2 commits into from
Feb 8, 2020
Merged

magento/magento2#: Unit test for \Magento\Bundle\Observer\AppendUpsellProductsObserver #26429

merged 2 commits into from
Feb 8, 2020

Conversation

atwixfirster
Copy link
Contributor

Description (*)

PR adds a unit test which cover \Magento\Bundle\Observer\AppendUpsellProductsObserver class.

Fixed Issues (if relevant)

  1. magento/magento2#<issue_number>: Issue title

Manual testing scenarios (*)

  1. ...
  2. ...

Questions or comments

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds are green)

Thank you!

@m2-assistant
Copy link

m2-assistant bot commented Jan 17, 2020

Hi @atwixfirster. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

  • @magento give me test instance - deploy test instance based on PR changes
  • @magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

@magento-engcom-team magento-engcom-team added Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner labels Jan 17, 2020
@dmytro-ch dmytro-ch self-assigned this Jan 17, 2020
lbajsarowicz
lbajsarowicz suggested changes Jan 17, 2020
View reviewed changes
Copy link
Contributor

@lbajsarowicz lbajsarowicz left a comment

Choose a reason for hiding this comment

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

Great job! Only choose more explanatory name for first test and we are done!

app/code/Magento/Bundle/Test/Unit/Observer/AppendUpsellProductsObserverTest.php Outdated
/**
* Test observer execute method
*/
public function testExecute()
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename to reflect what is given and what are the assertions.

Eg. "testExecuteForXLoadsCollectionOnce"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure :)

Thanks, @lbajsarowicz

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

app/code/Magento/Bundle/Test/Unit/Observer/AppendUpsellProductsObserverTest.php Outdated
$parentIds = [1];
$limit = 2;
$productId = 2;
$typeId = 'simple';
Copy link
Contributor

Choose a reason for hiding this comment

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

You can refer to const from Magento Core

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@ghost ghost assigned lbajsarowicz Jan 17, 2020
@atwixfirster atwixfirster force-pushed the unittest-AppendUpsellProductsObserverTest branch from b04a112 to d42d8fb Compare January 17, 2020 12:49
@atwixfirster
Copy link
Contributor Author

Great job! Only choose more explanatory name for first test and we are done!

Suggestions have been implemented.

Could you please review guys?

Many thanks!

@magento-engcom-team
Copy link
Contributor

Hi @lbajsarowicz, thank you for the review.
ENGCOM-6630 has been created to process this Pull Request
✳️ @lbajsarowicz, could you please add one of the following labels to the Pull Request?

Label Description
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required Changes in Pull Request does not require coverage by auto-tests

@engcom-Alfa engcom-Alfa self-assigned this Jan 17, 2020
@engcom-Alfa engcom-Alfa added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Jan 17, 2020
@engcom-Alfa
Copy link
Contributor

@atwixfirster Could you fix failed static tests?
Thanks!

@atwixfirster atwixfirster force-pushed the unittest-AppendUpsellProductsObserverTest branch from d42d8fb to c103950 Compare January 17, 2020 15:01
@atwixfirster
Copy link
Contributor Author

@atwixfirster Could you fix failed static tests?

fixed

Thanks for notice, @engcom-Alfa 👍

@ghost ghost removed the Progress: needs update label Jan 17, 2020
@magento-engcom-team
Copy link
Contributor

Hi @dmytro-ch, thank you for the review.
ENGCOM-6630 has been created to process this Pull Request

@lbajsarowicz
Copy link
Contributor

lbajsarowicz commented Jan 17, 2020 •
edited
Loading

Don't worry about Functional Tests - Fix will be merged soon to 2.4-develop, then you'll only have to merge latest 2.4-develop into your feature branch.

@atwixfirster
Copy link
Contributor Author

Don't worry about Functional Tests - Fix will be merged soon to 2.4-develop, then you'll only have to merge latest 2.4-develop into your feature branch.

Thank you for notice, @lbajsarowicz 🙇

@engcom-Alfa
Copy link
Contributor

✔️ QA Passed

@engcom-Golf engcom-Golf self-assigned this Jan 20, 2020
@engcom-Golf
Copy link
Contributor

failed Functional Tests B2B Not related to the changes in this PR

magento-engcom-team pushed a commit that referenced this pull request Feb 8, 2020
@slavvka
ENGCOM-6630: magento/magento2#: Unit test for \Magento\Bundle\Observe…
cfeb36f 
…r\AppendUpsellProductsObserver #26429
@magento-engcom-team magento-engcom-team merged commit ccc1823 into magento:2.4-develop Feb 8, 2020
@m2-assistant
Copy link

m2-assistant bot commented Feb 8, 2020

Hi @atwixfirster, 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lbajsarowicz lbajsarowicz lbajsarowicz approved these changes

@dmytro-ch dmytro-ch dmytro-ch approved these changes

@akaplya akaplya Awaiting requested review from akaplya

Assignees

@lbajsarowicz lbajsarowicz

@dmytro-ch dmytro-ch

@engcom-Alfa engcom-Alfa

@engcom-Golf engcom-Golf

Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: test coverage Component: Bundle Partner: Atwix Pull Request is created by partner Atwix partners-contribution Pull Request is created by Magento Partner Progress: accept Release Line: 2.4
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@atwixfirster @magento-engcom-team @engcom-Alfa @lbajsarowicz @engcom-Golf @dmytro-ch