Add gitlab ci and fix issues, prepare module for collaboration

Created on 18 May 2024, 7 months ago
Updated 28 July 2024, 4 months ago

Problem/Motivation

* Add gitlab ci-pipeline
* Fix issues that are reported
* Fix issues reported by HeikkiY
* Create a starting point for collaboration

📌 Task
Status

Fixed

Version

1.0

Component

Code

Created by

🇳🇱Netherlands joshahubbers

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @joshahubbers
  • Assigned to joshahubbers
  • Merge request !3Piwik pro 3448034 fix things → (Merged) created by 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 fixted

    Todo:
    * 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
  • 🇫🇮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

  • 🇫🇮Finland heikkiy Oulu

    Confirmed that all PHPUnit tests pass now.

  • 🇳🇱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 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?

  • 🇫🇮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

    Fine by me! Lets release.

  • 🇳🇱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
  • Pipeline finished with Skipped
    5 months ago
    #206986
  • Status changed to Fixed 5 months ago
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.71.5 2024