Skip to content

Fix failing / warnings in unit tests #28599

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
Merged
merged 9 commits into from
Aug 14, 2020
Merged

Fix failing / warnings in unit tests #28599

merged 9 commits into from
Aug 14, 2020

Conversation

lbajsarowicz
Copy link
Contributor

@lbajsarowicz lbajsarowicz commented Jun 7, 2020 •
edited
Loading

Description (*)

2.4.1-develop branch contains failing Unit tests - I've fixed that in scope of PR
Also - On 5th of June new version (9.2 of PHPUnit was released) I've upgraded it.

OK, but incomplete, skipped, or risky tests!
Tests: 15243, Assertions: 45840, Skipped: 87, Incomplete: 40.

Related Pull Requests

Fixed Issues (if relevant)

  1. Fixes magento/magento2#<issue_number>

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)

@m2-assistant
Copy link

m2-assistant bot commented Jun 7, 2020

Hi @lbajsarowicz. 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

❗ Automated tests can be triggered manually with an appropriate comment:

  • @magento run all tests - run or re-run all required tests against the PR changes
  • @magento run <test-build(s)> - run or re-run specific test build(s)
    For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names. Allowed build names are:

  1. Database Compare
  2. Functional Tests CE
  3. Functional Tests EE,
  4. Functional Tests B2B
  5. Integration Tests
  6. Magento Health Index
  7. Sample Data Tests CE
  8. Sample Data Tests EE
  9. Sample Data Tests B2B
  10. Static Tests
  11. Unit Tests
  12. WebAPI Tests

You can find more information about the builds here

ℹ️ Please run only needed test builds instead of all when developing. Please run all test builds before sending your PR for review.

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

@lbajsarowicz
Copy link
Contributor Author

@magento run all tests

@rogyar rogyar self-assigned this Jun 8, 2020
@rogyar rogyar added the Auto-Tests: Covered All changes in Pull Request is covered by auto-tests label Jun 8, 2020
rogyar
rogyar previously approved these changes Jun 8, 2020
View reviewed changes
Copy link
Contributor

@rogyar rogyar left a comment

Choose a reason for hiding this comment

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

@lbajsarowicz good job!

@magento-engcom-team
Copy link
Contributor

Hi @rogyar, thank you for the review.
ENGCOM-7632 has been created to process this Pull Request

@engcom-Alfa engcom-Alfa self-assigned this Jun 11, 2020
engcom-Alfa
engcom-Alfa requested changes Jun 11, 2020
View reviewed changes
Copy link
Contributor

@engcom-Alfa engcom-Alfa left a comment

Choose a reason for hiding this comment

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

Hi @lbajsarowicz . Could you resolve merge conflicts?
Thanks!

@ghost ghost dismissed rogyar’s stale review June 11, 2020 08:38

Pull Request state was updated. Re-review required.

@lbajsarowicz
Merge remote-tracking branch 'origin/2.4-develop' into feature/failed…
fd7cecb 
…-patch-information

# Conflicts:
#	composer.lock
@lbajsarowicz
Copy link
Contributor Author

I see that Magento Functional Testing Framework RC4 is coming, so probably we will need to resolve composer.lock a few more times.

composer update --with-dependencies allure-framework/allure-phpunit dealerdirect/phpcodesniffer-composer-installer friendsofphp/php-cs-fixer lusitanian/oauth magento/magento-coding-standard pdepend/pdepend phpcompatibility/php-compatibility phpmd/phpmd phpstan/phpstan phpunit/phpunit sebastian/phpcpd squizlabs/php_codesniffer

@lbajsarowicz
Copy link
Contributor Author

@magento run all tests

@lbajsarowicz lbajsarowicz changed the title Upgrade PHPUnit and fix failing / warnings in tests Upgrade PHPUnit 9.2 and fix failing / warnings in tests Jun 12, 2020
@lbajsarowicz
Merge remote-tracking branch 'origin/2.4-develop' into feature/failed…
17ae2d9 
…-patch-information

# Conflicts:
#	composer.lock
@lbajsarowicz
Copy link
Contributor Author

I am unable to resolve conflicts anymore.

I mean there is something introduced with regular dependencies which causes set of conflicts during composer update or composer require I'll revert the changes to composer.json and composer.lock leaving only fixes to tests.

@lbajsarowicz lbajsarowicz changed the title Upgrade PHPUnit 9.2 and fix failing / warnings in tests Fix failing / warnings in unit tests Jul 27, 2020
@lbajsarowicz
Copy link
Contributor Author

@magento run all tests

@lbajsarowicz
Copy link
Contributor Author

@magento run all tests

Fun fact: My PR is hanging for 2 months already, but most of the same changes performed last week (https://github.com/magento/magento2/pull/29244/files) were already merged.

What is more fun: The Magento 2.4 was released with the Unit Test bugs, even though the fix was already there 👏

@engcom-Echo engcom-Echo self-assigned this Aug 10, 2020
@engcom-Echo engcom-Echo requested a review from rogyar August 10, 2020 09:06
@magento-engcom-team
Copy link
Contributor

Hi @rogyar, thank you for the review.
ENGCOM-7632 has been created to process this Pull Request

@engcom-Alfa
Copy link
Contributor

QA not applicable

@engcom-Echo engcom-Echo added Priority: P3 May be fixed according to the position in the backlog. Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround. labels Aug 12, 2020
@engcom-Echo
Copy link
Contributor

@magento run all tests

magento-engcom-team pushed a commit that referenced this pull request Aug 14, 2020
@slavvka
ENGCOM-7632: Upgrade PHPUnit and fix failing / warnings in tests #28599
ed0a36b
@magento-engcom-team magento-engcom-team merged commit 9d5aa22 into magento:2.4-develop Aug 14, 2020
@m2-assistant
Copy link

m2-assistant bot commented Aug 14, 2020

Hi @lbajsarowicz, 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

@engcom-Alfa engcom-Alfa engcom-Alfa requested changes

@rogyar rogyar rogyar approved these changes

Assignees

@rogyar rogyar

@engcom-Alfa engcom-Alfa

@engcom-Echo engcom-Echo

Labels
Auto-Tests: Covered All changes in Pull Request is covered by auto-tests Award: category of expertise Component: CatalogImportExport Component: Developer Component: ProductVideo Component: Quote Partner: Blue Acorn iCi Partner: Mediotype partners-contribution Pull Request is created by Magento Partner Priority: P3 May be fixed according to the position in the backlog. Progress: accept Release Line: 2.4 Severity: S3 Affects non-critical data or functionality and does not force users to employ a workaround.
Projects
Pull Requests Dashboard
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@lbajsarowicz @magento-engcom-team @engcom-Alfa @engcom-Echo @rogyar