Skip to content

[#899] Enforced sorted imports with 'AlphabeticallySortedUses' and finished the backend rename in test names and 'MIGRATION.md'. - #903

Merged
AlexSkrypnyk merged 4 commits into
4.xfrom
feature/899-backend-loose-ends
Oct 2, 2026
Merged

AlexSkrypnyk merged 4 commits into
4.xfrom
feature/899-backend-loose-ends

Conversation

@AlexSkrypnyk

Copy link
Copy Markdown
Member

Closes #899

Summary

ahoy lint now fails on out-of-order use statements: phpcs.xml enables SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses, which compares namespaces segment by segment, case-insensitively, and ahoy lint-fix sorts whatever it flags. Alongside that, the "Drupal, Drush and Blackbox are backends, not drivers" section of MIGRATION.md now says which old behat_steps.driver* container names fail quietly and how to find them, and CoreCacheMethodsKernelTest keys its cache entries drupal_backend_test* instead of drupal_driver_test*.

Nothing checked import order, so when #896 renamed Driver to Backend each moved import had to be re-sorted by hand: 16 files had drifted, 6 of them from the rename, plus the nested-run context heredoc in BehatCliTrait.php and the behat --init template in ClassGenerator.php, which no sniff can read. The MIGRATION.md section also claimed "nothing keeps running against the old names by accident", yet an extension that sets a behat_steps.driver* parameter, tags its backend behat_steps.driver without listing it under backends, or guards an old service id with hasDefinition() gets no error at all.

After merge, an unsorted import fails ahoy lint with AlphabeticallySortedUses.IncorrectlyOrderedUses, and behat --init writes its context with sorted imports. No runtime code path changes: SubDriverFinderInterface stays removed, nothing rejects an old container name at build time, and rector.php and the before/after import pair in MIGRATION.md keep their order.

Before / After

Import order

BEFORE                                     AFTER
ahoy lint ─ phpcs                          ahoy lint ─ phpcs
├─ Drupal                                  ├─ Drupal
├─ DrevOps                                 ├─ DrevOps
├─ PHPCompatibility                        ├─ PHPCompatibility
└─ use order: unchecked                    └─ AlphabeticallySortedUses
   16 files + 5 embedded blocks               lint fails on drift,
   out of order                               ahoy lint-fix sorts it
A consumer extension still using an old container name

Old name used as                    Result              MIGRATION.md
                                                        before     after
──────────────────────────────────  ──────────────────  ─────────  ─────────
parameter behat_steps.driver.*      never read          covered    covered
@behat_steps.driver_registry arg    build fails         missing    covered
getDefinition() on an old id        build fails         missing    covered
hasDefinition() on an old id        FALSE, no error     missing    covered
behat_steps.driver tag, listed      build fails         missing    covered
behat_steps.driver tag, unlisted    backend left out    missing    covered

Section intro  before: "nothing keeps running against the old names by accident"
               after:  "The service container is the exception"

Changes

1. SubDriverFinderInterface stays removed

Nothing in src/ or tests/ still references SubDriverFinderInterface, getSubDriverPaths() or a SubBackend* name. The replacement MIGRATION.md gives type-checks: backendFor(CoreCapabilityInterface::class) returns a CoreCapabilityInterface, whose getCore() returns a CoreInterface declaring getExtensionPathList(). No change.

2. Test-only names follow the vocabulary

They stay renamed. A sweep of tests/, both fixture sites, scripts/, docs.php, behat*.php, the tool configs and the CI workflow found 2 spots #896 missed:

  • CoreCacheMethodsKernelTest keyed its cache entries and static as drupal_driver_test:sentinel, drupal_driver_test_counter and drupal_driver_test:memory. They're drupal_backend_test* now, matching driver_field_test becoming backend_field_test.
  • The comment on the nested run's 60-second timeout in BehatCliContext.php said "the 3.x DrupalDriver bootstraps Drupal in-process before any step runs". In 4.x the Drupal backend does the bootstrapping, and it happens when a capability first resolves to it, so the comment now names the backend and drops "before any step runs".

Every other "driver" left under tests/ is Mink's browser driver, or a test of the drivers key and @driver: tag rejections.

3. Old container names are documented, not rejected

There's no compiler pass. None of the behat_steps.driver* names ever shipped in a tagged release (git grep on 3.14.3 finds none), and a GitHub code search for behat_steps.driver finds no public consumer, so the issue's condition for adding one doesn't hold.

That makes MIGRATION.md the guard, and it needed fixing to do the job:

  • The section intro said "nothing keeps running against the old names by accident", which isn't true of the service container. It now says a parameter, service tag or service id under its old name can go unread without an error, and links to the details.
  • A service still tagged behat_steps.driver isn't registered, and the guide never said so. A backends list naming it fails the build with "which is not registered". Without a list, the backend is simply missing from the scenario's order, so each step resolves the first remaining backend that provides its capability. Both cases are written down now.
  • An old service id fails loudly as an @ argument or a getDefinition() call, and quietly behind a hasDefinition() check or as a service defined under the old id. The guide now says which is which.
  • A grep that matches every old container name: parameters, service ids and tags.

Each of those claims was checked against a real container build, not just read off the code.

4. Import order is linted

phpcs.xml enables SlevomatCodingStandard.Namespaces.AlphabeticallySortedUses at its defaults: segment by segment, case-insensitive, classes before functions before constants. That's the order most files already followed, so it flagged only 16 of the 456 files phpcs checks. The sniff ships with drupal/coder, which already enables 3 other Slevomat namespace sniffs. ahoy lint-fix sorted all 16, and that commit moves use lines and nothing else.

The sniff only reads real use statements, so every PHP string, Gherkin PyString, Markdown code fence and unlinted PHP file was scanned for embedded import blocks. The out-of-order ones were sorted by hand:

Where Why it matters
tests/behat/bootstrap/BehatCliTrait.php heredoc Writes the nested run's context
src/Behat/Generator/ClassGenerator.php template It's what behat --init writes into a consumer's project
tests/phpunit/src/Unit/Behat/Generator/ClassGeneratorTest.php Asserts that template's exact output
docs/usage.md A consumer-facing example the rename put out of order

2 are left alone on purpose. The "After" half of a before/after pair in MIGRATION.md keeps its lines mapped 1-to-1 onto the "Before" half, and rector.php sits outside phpcs's file list.

Worth a second look

  1. The behat --init template wasn't on the issue's list. Sorting it changes the import order of every context behat --init generates from now on, and nothing else.
  2. The BehatCliContext.php comment changed what it claims, not just a name: it no longer says Drupal bootstraps before any step runs.
  3. Nothing stops an embedded import block drifting out of order again. A parser for 5 blocks in 4 files felt like more machinery than the risk earns.

Testing

  • ahoy lint and ahoy lint-docs pass.
  • ahoy test-unit passes: 4,096 tests.
  • ahoy test-kernel passes: 125 tests.
  • ahoy test-bdd passes for the 3 features that cover the nested-run harness: behatcli.feature (9 scenarios), behatcli_steps.feature (4 scenarios) and drupal_watchdog.feature (21 scenarios). The last one runs a teardown hook that calls backendFor(CoreCapabilityInterface::class) from the context the re-sorted heredoc writes. The full suite runs in CI.

…emplate, the nested-run context heredoc and the 'docs/usage.md' example.
…_test' and dropped the 3.x 'DrupalDriver' from the nested-run timeout comment.
@coderabbitai

coderabbitai Bot commented Oct 2, 2026

Copy link
Copy Markdown

Warning

Review limit reached

  • Run on-demand review

This review includes 24 billable files and costs up to $6.00.

  • Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 24 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 91 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Team

Run ID: 18b232bc-70ab-4cbb-9533-1110e9a10f3d

📥 Commits

Reviewing files that changed from the base of the PR and between 79270fe and 6dae83c.

📒 Files selected for processing (24)
  • MIGRATION.md
  • docs.php
  • docs/usage.md
  • phpcs.xml
  • src/Behat/Generator/ClassGenerator.php
  • tests/behat/bootstrap/BehatCliContext.php
  • tests/behat/bootstrap/BehatCliTrait.php
  • tests/phpunit/src/Kernel/Backend/Core/CoreBlockMethodsKernelTest.php
  • tests/phpunit/src/Kernel/Backend/Core/CoreCacheMethodsKernelTest.php
  • tests/phpunit/src/Kernel/Backend/Core/CoreEntityCreateCommerceKernelTest.php
  • tests/phpunit/src/Kernel/Backend/Core/CoreEntityCreateModerationStateKernelTest.php
  • tests/phpunit/src/Kernel/Backend/Core/CoreEntityMethodsKernelTest.php
  • tests/phpunit/src/Kernel/Backend/Core/Field/FieldHandlerKernelTestBase.php
  • tests/phpunit/src/Kernel/Helper/Drupal/EntityLifecycleTraitVocabularyKernelTest.php
  • tests/phpunit/src/Unit/Backend/CoreLookupTest.php
  • tests/phpunit/src/Unit/Backend/DrushBackendResultTest.php
  • tests/phpunit/src/Unit/Behat/Generator/ClassGeneratorTest.php
  • tests/phpunit/src/Unit/Behat/Listener/BackendListenerTest.php
  • tests/phpunit/src/Unit/Behat/Manager/AuthenticatorTest.php
  • tests/phpunit/src/Unit/Behat/Manager/BackendRegistryTest.php
  • tests/phpunit/src/Unit/Behat/Prerequisite/PrerequisiteReaderTest.php
  • tests/phpunit/src/Unit/Behat/ServiceContainer/BackendPassTest.php
  • tests/phpunit/src/Unit/Helper/Drupal/EntityLifecycleTraitTest.php
  • tests/phpunit/src/Unit/Helper/Web/LastStepTraitTest.php
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


Comment @coderabbitai help to get the list of available commands.

@AlexSkrypnyk AlexSkrypnyk added this to the 4.0 milestone Oct 2, 2026
@AlexSkrypnyk
AlexSkrypnyk enabled auto-merge (squash) October 2, 2026 10:21
@AlexSkrypnyk AlexSkrypnyk added the AUTOMERGE Pull request has been approved and set to automerge label Oct 2, 2026
@codecov

codecov Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.96%. Comparing base (79270fe) to head (6dae83c).
⚠️ Report is 1 commits behind head on 4.x.

Additional details and impacted files
@@           Coverage Diff           @@
##              4.x     #903   +/-   ##
=======================================
  Coverage   95.96%   95.96%           
=======================================
  Files         164      164           
  Lines        8401     8401           
=======================================
  Hits         8062     8062           
  Misses        339      339           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@AlexSkrypnyk
AlexSkrypnyk merged commit 4c739a0 into 4.x Oct 2, 2026
@AlexSkrypnyk
AlexSkrypnyk deleted the feature/899-backend-loose-ends branch October 2, 2026 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AUTOMERGE Pull request has been approved and set to automerge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant