Make the gitlab-ci pipeline pass

Created on 20 October 2023, 8 months ago
Updated 23 January 2024, 5 months ago

Problem/Motivation

I've just added the gitlab-ci pipeline to the project. Tests run, but there are various fails in phpcs, eslint and test results.

Steps to reproduce

run the pipeline - see https://git.drupalcode.org/project/lupus_decoupled/-/pipelines/35154

Proposed resolution

Address test results.

📌 Task
Status

Fixed

Version

1.0

Component

Code

Created by

🇦🇹Austria fago Vienna

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

Merge Requests

Comments & Activities

  • Issue created by @fago
  • Status changed to Postponed 7 months ago
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    I'm afraid this is now a duplicate of 🐛 Fix PHPCS and maybe other errors Needs review which I opened to fix "some errors I am seeing" -- not realizing it was because of the recently added .gitlab-ci.yml.

    As far as I know, all issues are fixed in 🐛 Fix PHPCS and maybe other errors Needs review after custom_elements gets a new release. Let's see.

  • Status changed to Active 7 months ago
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    PHPCS / eslint / PHPStan (level 3) are passing now.

    This issue is now waiting to see if PHPUnit complaining about custom_elements is going to be resolved by it... or otherwise this issue will have to make sure to pull its -dev release in.

  • Status changed to Needs work 7 months ago
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    With the other issue merged and dependent modules having their config schema added:

    PHPUnit now shows one remaining failure in at least two issues' jobs, e.g. https://git.drupalcode.org/issue/lupus_decoupled-3320802/-/jobs/379582

    1) Drupal\Tests\lupus_decoupled\Functional\LupusDecoupledApiResponseTest::testExistingPageApiResponse
    Behat\Mink\Exception\ExpectationException: Current response status code is 404, but 200 expected.
    /builds/issue/lupus_decoupled-3320802/vendor/behat/mink/src/WebAssert.php:794
    /builds/issue/lupus_decoupled-3320802/vendor/behat/mink/src/WebAssert.php:130
    /builds/issue/lupus_decoupled-3320802/tests/src/Functional/LupusDecoupledApiResponseTest.php:76
    /builds/issue/lupus_decoupled-3320802/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
    ERRORS!
    Tests: 3, Assertions: 12, Errors: 1.
    HTML output was generated
    http://localhost/web/sites/simpletest/browser_output/Drupal_Tests_lupus_decoupled_Functional_LupusDecoupledApiResponseTest-1-17377110.html
    http://localhost/web/sites/simpletest/browser_output/Drupal_Tests_lupus_decoupled_Functional_LupusDecoupledApiResponseTest-2-68193535.html
    http://localhost/web/sites/simpletest/browser_output/Drupal_Tests_lupus_decoupled_Functional_LupusDecoupledApiResponseTest-3-36512604.html
    

    So that looks like it is present in the main branch / still needs to be investigated and fixed in this issue.

  • First commit to issue fork.
  • Merge request !69Consider path prefixes in middleware → (Closed) created by arthur_lorenz
  • 🇦🇹Austria arthur_lorenz Berlin

    After some debugging I'm confident that I have located the issue: The BackendApiRequest middleware is checking for the /ce-api prefix in the server's REQUEST_URI. Since the base url in gitlab ci is http://localhost/web, the actual uri that is checked at that point is /web/ce-api/node/1 which the middleware does not recognize as a valid ce api request and therefore passes the request through without altering it.

    I did not have time fix it yet though.

  • Status changed to Needs review 7 months ago
  • 🇦🇹Austria arthur_lorenz Berlin

    I fixed the middleware to work with path prefixes.

  • Status changed to Needs work 7 months ago
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    Oh joy, website install prefixes to shake out bugs!

    One comment.

  • Status changed to Needs review 7 months ago
  • 🇦🇹Austria arthur_lorenz Berlin

    Thx, good catch, I improved the string replacement.

  • Status changed to RTBC 7 months ago
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    Checked the string replacement thing. (Probably the first time I've consciously seen substr_replace(). Neat.)

    Apart from that: I feel confident RTBC'ing because the change is restricted to the internals of this one middleware, and the changes feel predictable.
    (Also, it's not trivial to make a test for the feedback I had earlier because the standard prefix is /ce-api.)

  • Status changed to Needs work 6 months ago
  • 🇦🇹Austria fago Vienna

    * The resulting pipeline does still not pass. Our fix is not fix the pipeline really, but installation with base-paths. Let's open an issue that clearly documents the fix we are doing, fix it there + keep this here open until we can really make it done - with green tests
    * Changes seem good, but this is very foundational, so we need to be 100% sure to not cause regressions here. That said, I think this misses test-coverage. Could we add a small unit-test here where we mock $request object, so we could fire it up with all kind of URI combinations and verify it's all correct? (--> in the other issue then)

  • 🇦🇹Austria arthur_lorenz Berlin

    The resulting pipeline does still not pass.

    I can not confirm this. The pipeline is passing

    Otherwise you are right, this should rather be handled and documented in a separate issue 🐛 Respect installations with base paths Needs review .

  • Status changed to Fixed 6 months ago
  • 🇦🇹Austria fago Vienna

    with the fix from that issue, the pipeline passed now! https://git.drupalcode.org/project/lupus_decoupled/-/pipelines/74214

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

Production build 0.69.0 2024