Create environment variable for the project's own folder

I was going to suggest this enhancement anyway, as it is helpful for anyone who is writing custom before_script and after_script. But another major advantage of having a variable, is that for D7 branches the path is slightly different -
$CI_PROJECT_DIR/$_WEB_ROOT/sites/all/modules/custom
So when cherry-picking commits across the branches in Gitlab Templates Downstream → having a variable to hold this path would mean we keep more of the template file aligned, rather than having to edit the path.

MR331 creates a new variable named $PROJECT_FOLDER but this can be changed if we think of a better name.

The phpstan and eslint GTD jobs now show

Executing "step_script" stage of the job script 00:07
$ cd $PROJECT_FOLDER && pwd
/builds/project/gitlab_templates_downstream/web/modules/custom/gitlab_templates_downstream

Ready for review and feedback (and more testing in other projects)

Since this is "the path of the project inside the Drupal site", maybe we can call it DRUPAL_PROJECT_FOLDER or DRUPAL_PROJECT_DIR. I know that we could add "drupal" to many variables (and I'm actually not a big fan of naming drupal-things with "drupal"), but in this case I think it makes a lot of sense.

PROJECT_FOLDER is kind of generic and could easily be confused with being the same as CI_PROJECT_DIR. The MR looks good, so I think it's just deciding on the name.

Good thinking, yes starting it with DRUPAL_PROJECT_ makes sense. Let's not use _DIR because that is too similar to CI_PROJECT_DIR. I have a slight preference for DRUPAL_PROJECT_FOLDER so will do that, and we can see how it reads in the code. But if you have a good reason for DRUPAL_PROJECT_PATH I can easily change it.

Also noting that this will need documenting somewhere.

Happy with DRUPAL_PROJECT_FOLDER. And yeah, +1 to mentioning it somewhere in the documentation.

I've updated the MR and added a before_script & after_script paragraph in 'customizations' with examples.

Will do full testing using Scheduler.

The MR looks really good. I made really minor suggestions to the documentation page to make it more explicit that they are just examples and what the purpose of the given examples might be for.

Full pipeline test https://git.drupalcode.org/issue/scheduler-3445052/-/pipelines/420834

Also, we still seem to have some references to the "old" composed path (look in the code for /custom. It'd be great to see if we can simplify those with this variable, if possible.

Yes you are right. I thought that the ones remaining were only where we had to use modules/custom without the project name. But there are some instances where the full path is still used, but it is relative, i.e. starts with $_WEB_ROOT, maybe that's why I missed them on the global search. I will change those if possible. Will need to test the find command which has a relative path.

I have pushed changes to composer-lint find, composer-base and phpunit when concurrent=0. Need to test these three.

The remaining places are:
D10
eslint makes links in the folder above $CI_PROJECT_NAME so that has to stay as-is

phpunit concurrent=1 uses --directory modules/custom/$CI_PROJECT_NAME - will need to test if this can be changed to absolute

upgrade status has

sed -i "s#modules\\\/custom\\\/$CI_PROJECT_NAME\\\/##" upgrade-status-analysis.json

phpcs

vendor/bin/phpcs -s $_WEB_ROOT/modules/custom --report-junit=junit.xml ...

This will all change in ✨ Add basepath parameter to PHP Code Sniffer Active so I was leaving it untouched until the full testing in that issue

D7 phpunit has

sudo SYMFONY_DEPRECATIONS_HELPER="$SYMFONY_DEPRECATIONS_HELPER" MINK_DRIVER_ARGS_WEBDRIVER="$MINK_DRIVER_ARGS_WEBDRIVER" -u www-data php $_WEB_ROOT/scripts/run-tests.sh --color --concurrency $_CONCURRENCY_THREADS --url "$SIMPLETEST_BASE_URL" --verbose --fail-only --xml "$BROWSERTEST_OUTPUT_DIRECTORY" --directory sites/all/modules/custom $_PHPUNIT_EXTRA || EXIT_CODE=$?

so that will need testing (not changed yet)

We can change the ones that we are sure about here and now, and for the others, we can either leave them as they are or create follow-ups.

As we are touching a good number of jobs, I triggered all downstream pipelines here: https://git.drupalcode.org/issue/gitlab_templates-3505585/-/pipelines/42...

I will try to review more in-depth tomorrow as I'm logging off earlier today.

I think the downstream API test failures are unrelated to this change. The tests run but one fails with

Drupal\Tests\api\Functional\LinkCodeTest::testApiLinkCode
    Linking to a PHP function
    Failed asserting that two strings are equal.
    --- Expected
    +++ Actual
    @@ @@
    -'strpos'
    +'strpos'

Suggestions to try to remove some more /custom/ around the code:

Calculate the destination path as: $DRUPAL_PROJECT_FOLDER/../

    # These links are created in the folder above modules/custom/$CI_PROJECT_NAME and will be used in addition to the project's own .eslintrc.json.
    - ln -s $CI_PROJECT_DIR/$_WEB_ROOT/core/.eslintrc.passing.json $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/.eslintrc.json
    - ln -s $CI_PROJECT_DIR/$_WEB_ROOT/core/.eslintrc.jquery.json $CI_PROJECT_DIR/$_WEB_ROOT/modules/custom/.eslintrc.jquery.json

Calculate the relative path to pass to run-test.sh. Not sure how to do it in bash but it'll be something like (untested)directory=${DRUPAL_PROJECT_FOLDER//"$CI_PROJECT_DIR/$_WEB_ROOT/"/}:

        # if _PHPUNIT_TESTGROUPS is --all then add --directory modules/custom/$CI_PROJECT_NAME
        # Otherwise add $_PHPUNIT_TESTGROUPS (without the --group)
        WHAT_TO_RUN=$([[ "$_PHPUNIT_TESTGROUPS" == "" ]] && echo "" || ([[ "$_PHPUNIT_TESTGROUPS" == "--all" ]] && echo "--directory modules/custom/$CI_PROJECT_NAME" || echo "$_PHPUNIT_TESTGROUPS"))

I'm thinking more an more to leave these "leftovers" for a follow-up issue, so I'm actually setting this to RTBC.

Created follow-up.

I think the downstream API test failures are unrelated to this change

Yes, probably due to 🐛 PHP function list download no longer exists Active . I know that seeing those red lights is not great but I still think that they are good projects to test things in. I need to find a gap and try to fix those tests for D7 (which is a new one) and D10.

Yes, I was going to ask if we want to do calculations on the new variable, and/or use .. . I can work on those in the follow, as this issue already has enough of the basic simple changes.

I have updated the issue summary with the actual job that shows each change. No all are covered yet, so I have put this back to NR until I have done the rest. But will return it to RTBC if nothing else is found that requires fixing.

All D10 changes have been checked - see links in issue summary.

I have checked the three changes in D7 and they work as expected. The .info file has

test_dependencies[] = date
test_dependencies[] = rules

and to force the 'install dependencies from .info' I deleted the project's composer.json, but oddly it could not install the date module. The error was

drush dl date
There are no stable releases for project date.                       [warning]
Choose one of the available releases for date:
 [0]  :  Cancel                                         
 [1]  :  7.x-3.x-dev     -  2023-Feb-01  -  Development 
 [2]  :  7.x-3.0-alpha2  -  2022-Sep-07  -  Recommended 
 [3]  :  7.x-2.x-dev     -  2023-Feb-01  -  Development 
 [4]  :  7.x-2.8         -  2014-Jul-29  -  Security    
 [5]  :  7.x-1.x-dev     -  2017-Mar-28  -  Development
Cancelled.         

But it runs fine with a composer.json containing

"require-dev": {
  "drupal/date": "*",
  "drupal/rules": "*"

But it not the fault of the changes in this MR, hence putting it back to RTBC as before.

That drush dl date issue where it offers options is one the reasons why D7-api is failing: https://git.drupalcode.org/project/api/-/jobs/4315997

I think it might be due to D7EOL actions on the modules I guess (no stable releases).

In any case, yes, not related to this issue.
--

Thanks for adding all the test cases in the issue description.

Merged!

Updated issue summary for D7

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