Adopt GitlabCi

Ready for review.

I've reviewed MR96, and it successfully adds the GIT CI template to the project. However, the pipeline flags numerous code style errors and failed PHPUnit tests. But these issues can be addressed in a follow-up task/issue.
Marking this issue to RTBC.
Thank you.

I disagree with the opinion in #4. This should not be committed until the PHPUnit failures are fixed. Otherwise all new MRs will begin failing for unrelated reasons. 26 out of the 29 tests are failing here with the following reason:

Drupal\Core\Config\UnmetDependenciesException: Configuration objects provided by inline_entity_form_test have unmet dependencies: field.field.node.ief_test_nested1.test_ref_nested1 (entity_reference)

So basically every test is broken when tested by GitLabCI even though they're passing on DrupalCI. Personally, I think that all unit test failures should be resolved before the GitLabCI configuration is committed. But this is definitely something that should not be fixed later!

There, now all of the unit tests are passing.

@Anjali thanks for review.

@dcam thanks for fixing the tests, I was not sure what module version is currently active. I find few places in config where entity_reference was used, I update configs that is going to change.

All changes looks good to me.

I applied suggested changes, needs new review.

I fixed the code style errors and warnings reported by PHPCS that were flagged during the last pipeline run.

@Anjali thanks for you contribution, but there is separate issue with fix for PHPcs violations 📌 Fix the issues reported by phpcs Needs work , let's keep it separate.

The changes in the MR visually look good upon review, and get the phpunit tests passing again. I'm seeing the failures locally, in DrupalCI and in GitLabCI. Frankly, we could split those out to a separate ticket and commit them, since they're orthogonal to the scope of adding GitLab CI. But at this point, let's just stop the bleeding. 😅 As evidenced by #2683125-56: Allow select widget for "Add existing items" → , merging this will get the test suite passing again everywhere. Bumping the priority and moving to RTBC. This is ready.

Thanks!
-Derek

Per @geek-merlin's comment in #3263281-19: Drupal 10 → , I turned the PHPStan level down to 1 to start with.

I won't take credit for this suggestion. @larowlan recommended that we do that same thing for the contrib Aggregator module, then turn the level up slowly over time. Some of the changes that PHPStan will recommend can result in BC breaks. So I sort of think of it as something to do when you're about ready for a new major version. But that's just my two cents.

I'm leaving the status as RTBC since this was a one-character change whose implementation was OKed by a maintainer.

Fantastic, thanks! I was going to suggest we do the same. However, even at level 1, the phpstan job is still failing. Let's open a follow-up for PHPStan and skip the job entirely for now (e.g. SKIP_PHPSTAN: 1 under variables).

Here's the follow-up: 📌 Get PHPStan level 1 passing and enable automated test job Active
Do you wanna push the change to .gitlab-ci.yml, and I'll re-RTBC?

Sure, I made the change.

Fantastic, thanks!

What a wonderful teamwork, esp. after so much disappointment the last months...!

Woot!

Yay, thanks!

Can we backport this to 8.x-1.x as a test-only improvement? Drupal CI is deprecated. We still want to be able to test 8.x-1.x until it’s unsupported.

FYI: At the end of the 8.x-1.x branch with both that and 3.x freshly pulled from origin, this works fine:

git cherry-pick a4b92d6db0251d6c9d50c4aaed4840f6ea7925b0

There's nothing to "port", we just need the commit in the other branch. Hence RTBC, not "Patch (to be ported)"...

Ok, then I won't work on doing a manual backport to 8.x-1.x unless I hear differently. But do you all want to add it to the D7 version? I would do it in order to be able to shut off the Drupal CI tests and save the association some money.

Of course, shutting of the Drupal CI integration means that all patches no longer work. They have to be converted to MRs. That hasn't been a big deal for me with my own modules, but for IEF that's a different matter. So I understand if you want to consider that carefully.

Very good points!

> We still want to be able to test 8.x-1.x until it’s unsupported.
Yup, committed and pushed.

> add it to the D7 version?
It's not my priority, but if s.o. wants to work on it... Is is just copy-paste or more complex?

> shutting of the Drupal CI integration means that all patches no longer work.
Oh, i was not aware of this. So imho we keep them running as long as they are supported.
If any module is worth this, IEF is among them...

Yeah, I don't care about the D7 version of IEF, either. If someone wants to maintain it, so be it. ;)

The GitLab CI config for D7 testing is pretty similar, but a little different. It shouldn't be too hard if we want it, but I don't think we can do a direct cherry-pick again, especially since this commit also included some fixes to the tests, and those almost certainly don't apply cleanly to the D7 branch.

Tentatively moving this back to Fixed. If someone really wants this for D7, I say they can open a child issue with a new MR for it. 😅

Thanks again!
-Derek

Ra Mänd identifies as D7 patriot...

Okay, then!

> shutting of the Drupal CI integration means that all patches no longer work.
Oh, i was not aware of this. So imho we keep them running as long as they are supported.

My statement here was a gross oversimplification. I apologize if it caused any confusion. Patches don't "stop working," they just stop being tested. The only means to test changes is through an MR.

Thus, if you want to enforce MRs, you can turn off Drupal CI tests. But then any pending patches must be recreated as MRs. That's probably a lot of work for a popular module. But it may be a good thing too. Let's say that we fix all PHPStan and PHPCS warnings. We can configure GitLab CI to fail any patch that doesn't pass them and enforce good standards. But if you continue to allow patches, then those checks won't be enforced.

So there are trade-offs. Personally, I would "rip the band-aid off" now and not look back. You should also know that you don't have to shut it off for every branch. "Turning off the tests" just means disabling the tests on the module's Automated Testing tab. So you could disable them for 8.x-1.x and 3.x, but leave them on for the D7 versions.

@dcam Yes OK! Me too tend to force MRs. Let's text some contribution rules in a new issue.