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

Tests: fix incorrect file and class name #20973

Merged
merged 4 commits into from Apr 15, 2024
Merged

Conversation

jrfnl
Copy link
Contributor

@jrfnl jrfnl commented Dec 19, 2023

Context

  • Code QA/consistency

Summary

This PR can be summarized in the following changelog entry:

  • Code QA/consistency

Relevant technical choices:

The test class wasn't running due to the incorrect file name (missing Test). Now the tests are being run, the tests were failing.

The first issue is fixed via fixing a typo in a property reference (x2).

However, there is a second issue:

1) Yoast\WP\SEO\Tests\Unit\Inc\Sitemaps\Sitemaps_Router_Test::test_template_redirect_exit
Mockery\Exception\InvalidCountException: Method home_url(<Any Arguments>) from Mockery_1 should be called
 exactly 1 times but called 2 times.

path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/CountValidator\Exact.php:38
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Expectation.php:310
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/ExpectationDirector.php:119
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Container.php:301
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Container.php:286
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/mockery/library/Mockery.php:210
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Adapter\Phpunit\MockeryPHPUnitIntegration.php:74
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Adapter\Phpunit\MockeryPHPUnitIntegration.php:49
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Adapter\Phpunit\MockeryPHPUnitIntegrationAssertPostConditionsForV8.php:29
C:\bin\phars\phpunit-9.6.15.phar:2326

Obviously, I can adjust the expectation, but I believe it should be checked that updating the expectation is actually the right course of action.

@vraja-pro Could you please have a look at this ?

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  • N/A No functional changes.

@jrfnl jrfnl added yoast cs/qa changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog labels Dec 19, 2023
@jrfnl jrfnl added this to the 21.9 milestone Dec 19, 2023
@jrfnl jrfnl force-pushed the JRF/tests/fix-file-class-name branch from 900abdd to 8bf03d4 Compare December 19, 2023 00:25
@jrfnl jrfnl requested a review from vraja-pro December 19, 2023 00:26
@jrfnl jrfnl force-pushed the JRF/tests/fix-file-class-name branch from 8bf03d4 to 686387c Compare December 19, 2023 07:25
@enricobattocchi enricobattocchi removed this from the 21.9 milestone Dec 29, 2023
@jrfnl jrfnl force-pushed the JRF/tests/fix-file-class-name branch 2 times, most recently from 41353d1 to 19bf6c1 Compare April 7, 2024 17:53
The test class wasn't running due to the incorrect file name (missing `Test`). Now the tests are being run, the tests were failing.

The first issue is fixed via fixing a typo in a property reference (x2).

However, there is a second issue:
```
1) Yoast\WP\SEO\Tests\Unit\Inc\Sitemaps\Sitemaps_Router_Test::test_template_redirect_exit
Mockery\Exception\InvalidCountException: Method home_url(<Any Arguments>) from Mockery_1 should be called
 exactly 1 times but called 2 times.

path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/CountValidator\Exact.php:38
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Expectation.php:310
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/ExpectationDirector.php:119
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Container.php:301
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Container.php:286
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/mockery/library/Mockery.php:210
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Adapter\Phpunit\MockeryPHPUnitIntegration.php:74
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Adapter\Phpunit\MockeryPHPUnitIntegration.php:49
path/to/wordpress-seo/vendor/mockery/mockery/library/Mockery/Adapter\Phpunit\MockeryPHPUnitIntegrationAssertPostConditionsForV8.php:29
C:\bin\phars\phpunit-9.6.15.phar:2326
```

Obviously, I can adjust the expectation, but I believe it should be checked that updating the expectation is actually the right course of action.
@jrfnl jrfnl force-pushed the JRF/tests/fix-file-class-name branch from 19bf6c1 to b78b58f Compare April 7, 2024 17:55
@jrfnl
Copy link
Contributor Author

jrfnl commented Apr 7, 2024

@vraja-pro I've rebased this PR. Could you have a look when you have the chance ?

@vraja-pro vraja-pro self-assigned this Apr 15, 2024
@vraja-pro
Copy link
Contributor

Fixed the test and other then that CR ✅

@vraja-pro vraja-pro merged commit b3251e7 into trunk Apr 15, 2024
22 checks passed
@vraja-pro vraja-pro deleted the JRF/tests/fix-file-class-name branch April 15, 2024 15:15
@coveralls
Copy link

Pull Request Test Coverage Report for Build da4920c65fa45f4e48638266e826df97464e3bdd

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on JRF/tests/fix-file-class-name at 47.517%

Totals Coverage Status
Change from base Build 1af6fa49d83d84b6f74ba6f81342ebeaaf77f537: 47.5%
Covered Lines: 15063
Relevant Lines: 31700

💛 - Coveralls

@vraja-pro vraja-pro removed their request for review April 16, 2024 06:45
@leonidasmi leonidasmi added this to the 22.6 milestone Apr 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: non-user-facing Needs to be included in the 'Non-userfacing' category in the changelog yoast cs/qa
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants