- Issue created by @joshahubbers
- Assigned to joshahubbers
- 🇳🇱Netherlands joshahubbers
Oops, pushed the gitlab-ci.yml file directly to the 1.0.x branch. But fixes will be done in this MR.
- 🇳🇱Netherlands joshahubbers
Done:
* gitlab ci added
* composer.json added
* .gitignore added
* cspell words list added
* test fixtedTodo:
* phpstan fixes - 🇳🇱Netherlands joshahubbers
Done:
* gitlab ci added
* composer.json added
* .gitignore added
* cspell words list added
* test fixted
* most phpstan fixes. Some phpstan errors are coming from hook-declarations that are inherited from core. - Assigned to heikkiy
- Status changed to Needs review
7 months ago 8:40pm 18 May 2024 - 🇫🇮Finland heikkiy Oulu
Thanks for the fixes!
The original test report was sent to @joshahubbers-0 through e-mail. The module was audited in collaboration with Cookie information who owns Piwik Pro currently. I will add the PHPStan and PHPUnit test results here so that we have the completely history also in Drupal.org.
The original audit was done for level 9 but that is probably too strict level because even core doesn't validate for that. Looking at https://github.com/mglaman/phpstan-drupal/blob/main/phpstan.neon it seems like level 8 would be good. I will test your changes and report any findings.
------ -----------------------------------------------------------------------------------------------------------------------
Line piwik_pro.module
------ -----------------------------------------------------------------------------------------------------------------------
17 Function piwik_pro_help() has no return type specified.
17 Function piwik_pro_help() has parameter $route_name with no type specified.
37 Function piwik_pro_page_attachments() has no return type specified.
37 Function piwik_pro_page_attachments() has parameter $attachments with no value type specified in iterable type array.
💡 See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iter...
56 Function piwik_pro_page_top() has no return type specified.
56 Function piwik_pro_page_top() has parameter $page with no type specified.
------ ----------------------------------------------------------------------------------------------------------------------------- --------------------------------------------------------------------------------------------------------------------------------------------------
Line src/Form/PiwikProAdminSettingsForm.php
------ --------------------------------------------------------------------------------------------------------------------------------------------------
23 Method Drupal\piwik_pro\Form\PiwikProAdminSettingsForm::getEditableConfigNames() return type has no value type specified in iterable type array.
💡 See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iter...
30 Method Drupal\piwik_pro\Form\PiwikProAdminSettingsForm::buildForm() has parameter $form with no value type specified in iterable type array.
💡 See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iter...
30 Method Drupal\piwik_pro\Form\PiwikProAdminSettingsForm::buildForm() return type has no value type specified in iterable type array.
💡 See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iter...
102 Method Drupal\piwik_pro\Form\PiwikProAdminSettingsForm::validateForm() has no return type specified.
102 Method Drupal\piwik_pro\Form\PiwikProAdminSettingsForm::validateForm() has parameter $form with no value type specified in iterable type array.
💡 See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iter...
106 Parameter #1 $string of function rtrim expects string, mixed given.
112 Parameter #1 $string of function strtolower expects string, mixed given.
118 Parameter #2 $subject of function preg_match expects string, mixed given.
123 Parameter #1 $string of function trim expects string, mixed given.
127 Parameter #2 $subject of function preg_split expects string, mixed given.
128 Argument of an invalid type array|false supplied for foreach, only iterables are supported.
140 Method Drupal\piwik_pro\Form\PiwikProAdminSettingsForm::submitForm() has no return type specified.
140 Method Drupal\piwik_pro\Form\PiwikProAdminSettingsForm::submitForm() has parameter $form with no value type specified in iterable type array.
💡 See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iter...
145 Parameter #1 $string of function substr expects string, mixed given.
------ -------------------------------------------------------------------------------------------------------------------------------------------------------- ----------------------------------------------------------------------------------------------------------------------
Line src/PiwikProSnippet.php
------ ----------------------------------------------------------------------------------------------------------------------
83 Method Drupal\piwik_pro\PiwikProSnippet::getScript() return type has no value type specified in iterable type array.
💡 See: https://phpstan.org/blog/solving-phpstan-no-value-type-specified-in-iter...
97 Parameter #2 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
97 Parameter #3 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
97 Parameter #4 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
100 Parameter #2 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
100 Parameter #3 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
109 Method Drupal\piwik_pro\PiwikProSnippet::getScript() should return array but returns null.
131 Parameter #2 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
131 Parameter #3 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
131 Parameter #4 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
137 Method Drupal\piwik_pro\PiwikProSnippet::getSyncScript() should return string but returns null.
145 Method Drupal\piwik_pro\PiwikProSnippet::getVisibilityPages() has no return type specified.
155 Parameter #1 $string of function mb_strtolower expects string, mixed given.
------ ---------------------------------------------------------------------------------------------------------------------------- ----------------------------------------------------------------------------------------------------------------------------------------------------------------
Line tests/src/Functional/PiwikProSnippetTest.php
------ ----------------------------------------------------------------------------------------------------------------------------------------------------------------
53 Property Drupal\Tests\piwik_pro\Functional\PiwikProSnippetTest::$config (Drupal\Core\Config\ImmutableConfig) does not accept Drupal\Core\Config\Config.
71 Parameter #1 $account of method Drupal\Tests\BrowserTestBase::drupalLogin() expects Drupal\Core\Session\AccountInterface, Drupal\user\Entity\User|false given.
80 Method Drupal\Tests\piwik_pro\Functional\PiwikProSnippetTest::testPiwikProConfiguration() has no return type specified.
93 Method Drupal\Tests\piwik_pro\Functional\PiwikProSnippetTest::testPiwikProHelp() has no return type specified.
96 Call to an undefined method Drupal\Tests\WebAssert::pageTextContains().
100 Call to an undefined method Drupal\Tests\WebAssert::pageTextContains().
106 Method Drupal\Tests\piwik_pro\Functional\PiwikProSnippetTest::testSnippetVisibility() has no return type specified.
110 Parameter #2 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
111 Parameter #3 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
113 Parameter #1 $text of method Drupal\Tests\WebAssert::responseContains() expects object|string, mixed given.
117 Parameter #2 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
122 Parameter #1 $text of method Drupal\Tests\WebAssert::responseNotContains() expects object|string, mixed given.
128 Method Drupal\Tests\piwik_pro\Functional\PiwikProSnippetTest::testSnippetVisibilityPagesInverted() has no return type specified.
133 Parameter #1 $text of method Drupal\Tests\WebAssert::responseNotContains() expects object|string, mixed given.
136 Parameter #1 $text of method Drupal\Tests\WebAssert::responseContains() expects object|string, mixed given.
142 Method Drupal\Tests\piwik_pro\Functional\PiwikProSnippetTest::testSnippetVisibilitySyncSnippet() has no return type specified.
148 Parameter #2 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
155 Method Drupal\Tests\piwik_pro\Functional\PiwikProSnippetTest::testSnippetVisibilityChangedDataLayer() has no return type specified.
162 Parameter #3 ...$values of function sprintf expects bool|float|int|string|null, mixed given.
169 Method Drupal\Tests\piwik_pro\Functional\PiwikProSnippetTest::testSnippetVisibilityInvalidSettings() has no return type specified.
205 Method Drupal\Tests\piwik_pro\Functional\PiwikProSnippetTest::testSettingsForm() has no return type specified.
207 Call to an undefined method Drupal\Tests\WebAssert::statusCodeEquals().
208 Call to an undefined method Drupal\Tests\WebAssert::pageTextContains().
209 Call to an undefined method Drupal\Tests\WebAssert::elementExists().
------ ----------------------------------------------------------------------------------------------------------------------------------------------------------------[ERROR] Found 57 errors
Also the PHPUnit test results were the following
Piwik Pro Snippet (Drupal\Tests\piwik_pro\Functional\PiwikProSnippet)
✔ Piwik pro configuration
✘ Piwik pro help
│
│ Behat\Mink\Exception\ResponseTextException: The text "Piwik Pro is a GDPR-proof tracking tool that allows you to track user visits." was not found anywhere in the text of the current page.
│
│ /var/www/html/vendor/behat/mink/src/WebAssert.php:907
│ /var/www/html/vendor/behat/mink/src/WebAssert.php:293
│ /var/www/html/tests/src/Functional/PiwikProSnippetTest.php:100
│ /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
│✔ Snippet visibility
✔ Snippet visibility pages inverted
✔ Snippet visibility sync snippet
✔ Snippet visibility changed data layer
✔ Snippet visibility invalid settings
✘ Settings form
│
│ TypeError: Behat\Mink\Element\TraversableElement::findButton(): Argument #1 ($locator) must be of type string, Drupal\Core\StringTranslation\TranslatableMarkup given, called in /var/www/html/web/core/tests/Drupal/Tests/WebAssert.php on line 143
│
│ /var/www/html/vendor/behat/mink/src/Element/TraversableElement.php:97
│ /var/www/html/web/core/tests/Drupal/Tests/WebAssert.php:143
│ /var/www/html/web/core/tests/Drupal/Tests/UiHelperTrait.php:78
│ /var/www/html/tests/src/Functional/PiwikProSnippetTest.php:215
│ /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
│Time: 00:13.857, Memory: 4.00 MB
Summary of non-successful tests:
Piwik Pro Snippet (Drupal\Tests\piwik_pro\Functional\PiwikProSnippet)
✘ Piwik pro help
│
│ Behat\Mink\Exception\ResponseTextException: The text "Piwik Pro is a GDPR-proof tracking tool that allows you to track user visits." was not found anywhere in the text of the current page.
│
│ /var/www/html/vendor/behat/mink/src/WebAssert.php:907
│ /var/www/html/vendor/behat/mink/src/WebAssert.php:293
│ /var/www/html/tests/src/Functional/PiwikProSnippetTest.php:100
│ /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
│✘ Settings form
│
│ TypeError: Behat\Mink\Element\TraversableElement::findButton(): Argument #1 ($locator) must be of type string, Drupal\Core\StringTranslation\TranslatableMarkup given, called in /var/www/html/web/core/tests/Drupal/Tests/WebAssert.php on line 143
│
│ /var/www/html/vendor/behat/mink/src/Element/TraversableElement.php:97
│ /var/www/html/web/core/tests/Drupal/Tests/WebAssert.php:143
│ /var/www/html/web/core/tests/Drupal/Tests/UiHelperTrait.php:78
│ /var/www/html/tests/src/Functional/PiwikProSnippetTest.php:215
│ /var/www/html/vendor/phpunit/phpunit/src/Framework/TestResult.php:729
│ERRORS!
Tests: 8, Assertions: 75, Errors: 2.
Failed to run phpunit : exit status 2 - 🇳🇱Netherlands joshahubbers
Hi @HeikkiY.
Sorry for my absense. Had a very busy time lately. Switched work and also personally. I will look at your comments as soon as possible, but some quick responses:
* I run the tests with the ddev tooling
* git instructions are updated automatically on d.org normally as soon as one starts committing in the repo. Strange that it doesn't here.
* I agree that we can start a new major branch, and drop support for older Drupal versions. I would suggest only supporting the currently supported versions in the new branchMaybe a good start to commit the fixes in this MR, because most of the issues are solved. And than we move to a new 2.x branch to start working on a new release?
- 🇫🇮Finland heikkiy Oulu
Agreed. We can review these with @kekkis and approve the MR as soon as possible.
- 🇫🇮Finland heikkiy Oulu
@joshahubbers-0 Can you change the MR from Draft to an actual MR?
Also there are a couple of comments to the MR which would be low hanging fruits like adding the core requirement and PHP requirement to composer.json.
- 🇳🇱Netherlands joshahubbers
I had the same reasoning as you. If someone is on 8.x, then I don't think they will be doing minor modules updates either...
- Status changed to RTBC
5 months ago 4:04pm 24 June 2024 -
HeikkiY →
committed dab13cca on 1.0.x authored by
JoshaHubbers →
Issue #3448034 by JoshaHubbers, HeikkiY: Add gitlab ci and fix issues,...
-
HeikkiY →
committed dab13cca on 1.0.x authored by
JoshaHubbers →
- Status changed to Fixed
5 months ago 1:36pm 14 July 2024 Automatically closed - issue fixed for 2 weeks with no activity.