Problem/Motivation

Not yet using Gitlab CI.

Steps to reproduce

Proposed resolution

Start using Gitlab CI.

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork upgrade_status-3403194

Command icon Show commands

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:

About issue forks

Comments

Gábor Hojtsy created an issue. See original summary.

marvil07 opened merge request !59

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

The single Drupal 10 fail is due to placement of the module:

There was 1 failure:
1) Drupal\Tests\upgrade_status\Kernel\TwigDeprecationAnalyzerTest::testDeprecationReport
Failed asserting that two objects are equal.
--- Expected
+++ Actual
@@ @@
 Drupal\upgrade_status\DeprecationMessage Object (
     'message' => 'Twig Filter "deprecatedfilter...71078.'
     'line' => 10
-    'file' => 'modules/contrib/upgrade_status/tests/modules/upgrade_status_test_twig/templates/test.html.twig'
+    'file' => 'modules/custom/upgrade_status-3403194/tests/modules/upgrade_status_test_twig/templates/test.html.twig'
 )
/builds/issue/upgrade_status-3403194/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
/builds/issue/upgrade_status-3403194/tests/src/Kernel/TwigDeprecationAnalyzerTest.php:32
/builds/issue/upgrade_status-3403194/vendor/phpunit/phpunit/src/Framework/TestResult.php:728

While after merging that the test will work, it will not work for future MRs either, so we need to fix the assertion to be more MR-proof.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Needs work

The 9 pipeline had similar fails, but uncovers one more fail point:

There were 2 failures:
1) Drupal\Tests\upgrade_status\Functional\UpgradeStatusAnalyzeTest::testAnalyzer
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'The spaceless tag in "modules/contrib/upgrade_status/tests/modules/upgrade_status_test_twig/templates/spaceless.html.twig" at line 2 is deprecated since Twig 2.7, use the "spaceless" filter with the "apply" tag instead. See https://drupal.org/node/3071078.'
+'The spaceless tag in "modules/custom/upgrade_status-3403194/tests/modules/upgrade_status_test_twig/templates/spaceless.html.twig" at line 2 is deprecated since Twig 2.7, use the "spaceless" filter with the "apply" tag instead. See https://drupal.org/node/3071078.'
/builds/issue/upgrade_status-3403194/vendor/phpunit/phpunit/src/Framework/Constraint/Equality/IsEqual.php:94
/builds/issue/upgrade_status-3403194/tests/src/Functional/UpgradeStatusAnalyzeTest.php:143
/builds/issue/upgrade_status-3403194/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
2) Drupal\Tests\upgrade_status\Kernel\TwigDeprecationAnalyzerTest::testDeprecationReport
Failed asserting that an array contains Drupal\upgrade_status\DeprecationMessage Object &0000000040424a40000000000cf51e80 (
    'message' => 'Twig Filter "deprecatedfilter" is deprecated. See https://drupal.org/node/3071078.'
    'line' => 10
    'file' => 'modules/contrib/upgrade_status/tests/modules/upgrade_status_test_twig/templates/test.html.twig'
).
/builds/issue/upgrade_status-3403194/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:121
/builds/issue/upgrade_status-3403194/vendor/phpunit/phpunit/src/Framework/Constraint/Constraint.php:55
/builds/issue/upgrade_status-3403194/tests/src/Kernel/TwigDeprecationAnalyzerTest.php:37
/builds/issue/upgrade_status-3403194/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
FAILURES!

marvil07 made their first commit to this issue’s fork.

marvil07’s picture

marvil07 commented
Status: Needs work » Needs review

@Gábor Hojtsy, it took a few iterations to get it right, but I think the current code in the merge request is now ready for inclusion.

At tests/modules/upgrade_status_test_error/fatal.php, I added a couple of comments, one for php linting, and the other for coding standards.
The linting one is the important, that avoids a lint fail, which is expected in that case.
To avoid it I declared that it should be only linted on versions lower than php 7, and given composer requires higher than php 7 indirectly via dependencies, it basically will not happen ever.

That last bit needed some changes on UpgradeStatusAnalyzeTest, since a few messages there contain the line number of the parse error.

Finally, the directory assumptions on test, as you mentioned on #3 and #4, needed a few changes.
To solve it, I am relying on the extension object related to the module where the tested template lives.

There are multiple suggestions from phpcs, phpstan, and stylint, but they are all optional, so I have not really dive into them.

Marking now as NR.

  • 7b3694c2 committed on 4.x 
    Issue #3403194 by marvil07, Gábor Hojtsy: Adopt gitlab CI for testing
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Landed in 4.x, thanks for the fantastic work on this!

Now trying to backport to 3.x. I think the getPath() may need to be rewritten as drupal_get_path(), but let's make the tests tell us that :D

Gábor Hojtsy opened merge request !61

  • 1de81956 committed on 8.x-3.x 
    Issue #3403194 by marvil07, Gábor Hojtsy: Adopt gitlab CI for testing
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Version: 4.0.0 » 8.x-3.x-dev
Status: Needs review » Fixed

I checked and the extension class based path methods are already used, so that should be fine. Committed to 3.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.