Closed (fixed)
Project:
Piwik PRO
Version:
1.0.2
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
18 May 2024 at 16:49 UTC
Updated:
28 Jul 2024 at 13:39 UTC
Jump to comment: Most recent
* Add gitlab ci-pipeline
* Fix issues that are reported
* Fix issues reported by HeikkiY
* Create a starting point for collaboration
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
joshahubbers commentedComment #4
joshahubbers commentedOops, pushed the gitlab-ci.yml file directly to the 1.0.x branch. But fixes will be done in this MR.
Comment #5
joshahubbers commentedDone:
* gitlab ci added
* composer.json added
* .gitignore added
* cspell words list added
* test fixted
Todo:
* phpstan fixes
Comment #6
joshahubbers commentedDone:
* 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.
Comment #7
joshahubbers commentedComment #8
heikkiy commentedThanks 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
Comment #9
heikkiy commentedConfirmed that all PHPUnit tests pass now.
Comment #10
joshahubbers commentedHi @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 branch
Maybe 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?
Comment #11
heikkiy commentedAgreed. We can review these with @kekkis and approve the MR as soon as possible.
Comment #12
heikkiy commented@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.
Comment #13
joshahubbers commentedFine by me! Lets release.
Comment #14
joshahubbers commentedI 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...
Comment #15
joshahubbers commentedComment #17
joshahubbers commented