Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
While working on #3151732: DrupalSqlBase::checkRequirements should test version with $minimum_version, we noticed some things in the test class Drupal\Tests\migrate_drupal\Unit\source\DrupalSqlBase that should be cleaned up.
Before that issue, there was already some duplicated code between the two test methods. Now there are three test methods that have this similar code.
Steps to reproduce
Proposed resolution
- Reduce duplicated code in the test methods. Either add one or more helper function or a
setUp()method. - Remove the second
namespacedeclaration beforeTestDrupalSqlBase. - Combine all the
usestatements at the start of the file.
Remaining tasks
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
N/A
Issue fork drupal-3191990
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
- 3191990-simplify-code-in
changes, plain diff MR !325
Comments
Comment #2
quietone commentedComment #4
quietone commentedComment #5
marvil07 commented@quietone Thanks for the changes
The changes here look good, and indeed simplify the codebase.
Overall this look good, I only added docblock suggestion to be a bit more accurate, but apart from it, it feels ready.
Comment #6
quietone commentedComment #7
marvil07 commented@quietone Thanks for the extra change!
Comment #8
quietone commented@marvil07, and to you for the review.
Comment #10
catchCommitted 768340b and pushed to 9.2.x. Thanks!
Comment #11
wim leers