Navigation Menu

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

Update container factory to allow for sorting plugins #18074

Merged
merged 2 commits into from Sep 30, 2021
Merged

Conversation

JasonMortonNZ
Copy link
Contributor

Description:

Issue: dev-2082

This PR updates a test helper class and the container factory to enable plugin sorting. This is required for cloud plugins.

Review

@JasonMortonNZ JasonMortonNZ added the Needs Review PRs that need a code review label Sep 29, 2021
@JasonMortonNZ JasonMortonNZ added this to the 4.5.0 milestone Sep 29, 2021
@tsteur
Copy link
Member

tsteur commented Sep 29, 2021

@JasonMortonNZ I just tested this and the tracking spam tests in cloud still didn't work for me.

I then noticed that when running eg ./console tests:run plugins/Cloud/tests/Integration/TrackingSpamPrevention/CachedIPRangeProviderTests.php the GLOBALS sort function is set correctly. This then launches a phpunit command /Users/.../vendor/bin/phpunit ../../plugins/Cloud/tests/Integration/TrackingSpamPrevention/CachedIPRangeProviderTests.php where then the GLOBALS from bootstrap.php is not defined. Only when I configure the PHPUnit/bootstrap.php to also load the $matomoDir/bootstraph.php then the tests pass for me.

diff --git a/tests/PHPUnit/bootstrap.php b/tests/PHPUnit/bootstrap.php
index f74696e954..e4d75e6e17 100644
--- a/tests/PHPUnit/bootstrap.php
+++ b/tests/PHPUnit/bootstrap.php
@@ -25,6 +25,10 @@ if (!defined('PIWIK_INCLUDE_PATH')) {
     define('PIWIK_INCLUDE_PATH', PIWIK_PATH_TEST_TO_ROOT);
 }
 
+if (file_exists(PIWIK_DOCUMENT_ROOT . '/bootstrap.php')) {
+    require_once PIWIK_DOCUMENT_ROOT . '/bootstrap.php';
+}

Can you confirm this?

@JasonMortonNZ
Copy link
Contributor Author

Can you confirm this?

Yes, I've just retested this and confirm the same. I have to require_once the bootstrap.php file in order for this to pass.

@tsteur tsteur merged commit 236d27c into next_release Sep 30, 2021
@tsteur tsteur deleted the dev-2082-v2 branch September 30, 2021 02:55
@justinvelluppillai justinvelluppillai changed the title [Dev-2082] Update container factory to allow for sorting plugins Update container factory to allow for sorting plugins Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review PRs that need a code review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants