Skip to content

Removed the dead drush stub from 'TestDrushDriver' and documented the legacy Drush removal. - #390

Merged
AlexSkrypnyk merged 1 commit into
masterfrom
feature/drush-legacy-cleanup
Jul 28, 2026
Merged

Removed the dead drush stub from 'TestDrushDriver' and documented the legacy Drush removal.#390
AlexSkrypnyk merged 1 commit into
masterfrom
feature/drush-legacy-cleanup

Conversation

@AlexSkrypnyk

@AlexSkrypnyk AlexSkrypnyk commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

This is a follow-up to #387, which deleted DrushDriver's Drush 8 compatibility layer (the isLegacyDrush() probe, the protected static bool $isLegacyDrush cache, the legacy cacheClear() branch, and the --nocolor vs --no-ansi fork). Review of that PR flagged two things it left behind: a test double in DrushDriverTest.php that only existed to feed the now-deleted testIsLegacyDrush, and a gap in UPGRADING.md around the two removed protected members, which this project treats as a downstream subclassing surface. This PR removes the dead test stub and adds the missing upgrade-notes entry.

Changes

  • tests/Drupal/Tests/Driver/Unit/DrushDriverTest.php: Removed TestDrushDriver::$drushOutput and its drush() override. Both were added solely to stub drush() for testIsLegacyDrush, which remove legacyDrush check #387 already deleted; the remaining testParseUserId only calls callParseUserId(), pure string parsing that never reaches drush().
  • UPGRADING.md: Added a "DrushDriver no longer detects legacy Drush" entry under the v3 removal notes, documenting that DrushDriver::isLegacyDrush() and DrushDriver::$isLegacyDrush are gone, and that drushResult(), cacheClear(), and bootstrap() now run their previously-conditional Drush 9+ behaviour unconditionally.

Before / After

tests/Drupal/Tests/Driver/Unit/DrushDriverTest.php
──────────────────────────────────────────────────

┌─ BEFORE ───────────────────────────────────────────────────┐
│ class TestDrushDriver extends DrushDriver {
│
│   public string $drushOutput = '';
│   // nothing writes to this since #387 deleted
│   // testIsLegacyDrush
│
│   public function drush($command, array $arguments = [],
│       array $options = []): string {
│     return $this->drushOutput;
│   }
│
│   public function callParseUserId(string $info): ?int { ... }
│ }
└────────────────────────────────────────────────────────────┘

┌─ AFTER ────────────────────────────────────────────────────┐
│ class TestDrushDriver extends DrushDriver {
│
│   public function callParseUserId(string $info): ?int { ... }
│ }
└────────────────────────────────────────────────────────────┘

UPGRADING.md (v3 removal notes)
───────────────────────────────

┌─ BEFORE ───────────────────────────────────────────────────┐
│ ...
│ ### DrushDriver no longer supports Content or Field capabilities
│ ### CoreInterface expanded
│ ...
└────────────────────────────────────────────────────────────┘

┌─ AFTER ────────────────────────────────────────────────────┐
│ ...
│ ### DrushDriver no longer supports Content or Field capabilities
│ ### DrushDriver no longer detects legacy Drush        (added)
│   - DrushDriver::isLegacyDrush() (protected) - removed.
│   - DrushDriver::$isLegacyDrush (protected static) - removed.
│ ### CoreInterface expanded
│ ...
└────────────────────────────────────────────────────────────┘

Summary by CodeRabbit

  • Documentation
    • Added upgrade guidance for changes to Drush integration behavior.
    • Clarified that bootstrapping no longer probes the Drush version.
    • Documented consistent command options and cache-rebuild behavior across supported Drush versions.
    • Noted that custom subclasses overriding legacy Drush detection may require updates.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6391e98b-f08c-4cd4-8890-bf9360c639ba

📥 Commits

Reviewing files that changed from the base of the PR and between 0811772 and 97ae836.

📒 Files selected for processing (2)
  • UPGRADING.md
  • tests/Drupal/Tests/Driver/Unit/DrushDriverTest.php

📝 Walkthrough

Walkthrough

The pull request documents the removal of legacy Drush detection and removes obsolete Drush stubbing from the unit-test subclass.

Changes

Legacy Drush update

Layer / File(s) Summary
Legacy Drush documentation and test fixture cleanup
UPGRADING.md, tests/Drupal/Tests/Driver/Unit/DrushDriverTest.php
Documents unconditional Drush behavior and required subclass updates, while removing the test subclass’s stored output property and drush() stub.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

  • jhedstrom/DrupalDriver#387: Removes or stops exercising related legacy Drush detection, flag handling, and behavior expectations.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the two main changes: removing the dead TestDrushDriver drush stub and documenting the legacy Drush removal.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/drush-legacy-cleanup

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions

Copy link
Copy Markdown

Code coverage (threshold: 95%)



Code Coverage Report Summary:
  Classes: 58.82% (20/34)
  Methods: 89.47% (221/247)
  Lines:   96.24% (1229/1277)

Per-class coverage
Drupal\Driver\Alias\CreationAliasRegistryTrait               100.00%
Drupal\Driver\Alias\RolesAlias                                87.50%
Drupal\Driver\BlackboxDriver                                 100.00%
Drupal\Driver\Core\Alias\AuthorAlias                          90.48%
Drupal\Driver\Core\Alias\ParentTermAlias                      88.24%
Drupal\Driver\Core\Alias\VocabularyMachineNameAlias           88.89%
Drupal\Driver\Core\Core                                       95.81%
Drupal\Driver\Core\Field\AbstractHandler                      95.74%
Drupal\Driver\Core\Field\AddressHandler                       94.83%
Drupal\Driver\Core\Field\BooleanHandler                      100.00%
Drupal\Driver\Core\Field\ColorFieldTypeHandler                 0.00%
Drupal\Driver\Core\Field\DateRecurHandler                    100.00%
Drupal\Driver\Core\Field\DaterangeHandler                    100.00%
Drupal\Driver\Core\Field\DatetimeHandler                     100.00%
Drupal\Driver\Core\Field\DefaultHandler                      100.00%
Drupal\Driver\Core\Field\EmbridgeAssetItemHandler              0.00%
Drupal\Driver\Core\Field\EntityReferenceHandler               94.44%
Drupal\Driver\Core\Field\EntityReferenceRevisionsHandler      86.05%
Drupal\Driver\Core\Field\FieldClassifier                      94.44%
Drupal\Driver\Core\Field\FieldShapeClassifier                100.00%
Drupal\Driver\Core\Field\FileHandler                         100.00%
Drupal\Driver\Core\Field\ImageHandler                        100.00%
Drupal\Driver\Core\Field\LinkHandler                          94.00%
Drupal\Driver\Core\Field\ListFloatHandler                    100.00%
Drupal\Driver\Core\Field\ListHandlerBase                     100.00%
Drupal\Driver\Core\Field\ListIntegerHandler                    0.00%
Drupal\Driver\Core\Field\ListStringHandler                     0.00%
Drupal\Driver\Core\Field\NameHandler                          96.36%
Drupal\Driver\Core\Field\OgStandardReferenceHandler            0.00%
Drupal\Driver\Core\Field\SmartdateHandler                    100.00%
Drupal\Driver\Core\Field\SupportedImageHandler               100.00%
Drupal\Driver\Core\Field\TextHandler                           0.00%
Drupal\Driver\Core\Field\TextLongHandler                       0.00%
Drupal\Driver\Core\Field\TextWithSummaryHandler                0.00%
Drupal\Driver\Core\Field\TimeHandler                          91.67%
Drupal\Driver\DrupalDriver                                    98.86%
Drupal\Driver\Drush\DrushResult                              100.00%
Drupal\Driver\DrushDriver                                    100.00%
Drupal\Driver\Entity\EntityStub                              100.00%
Drupal\Driver\Exception\BootstrapException                   100.00%
Drupal\Driver\Exception\CreationAliasResolutionException       0.00%
Drupal\Driver\Exception\Exception                            100.00%
Drupal\Driver\Exception\UnsupportedDriverActionException     100.00%

@AlexSkrypnyk
AlexSkrypnyk merged commit c7dee2d into master Jul 28, 2026
13 checks passed
@AlexSkrypnyk
AlexSkrypnyk deleted the feature/drush-legacy-cleanup branch July 28, 2026 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant