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 namespace declaration before TestDrupalSqlBase.
  • Combine all the use statements 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

Command icon 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:

Comments

benjifisher created an issue. See original summary.

quietone’s picture

Title: [PP-1] Simplify code in DrupalSqlBase » Simplify code in DrupalSqlBase

quietone’s picture

Status: Active » Needs review
marvil07’s picture

Status: Needs review » Needs work

@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.

quietone’s picture

Status: Needs work » Needs review
marvil07’s picture

Status: Needs review » Reviewed & tested by the community

@quietone Thanks for the extra change!

quietone’s picture

@marvil07, and to you for the review.

  • catch committed 768340b on 9.2.x
    Issue #3191990 by quietone, marvil07, benjifisher: Simplify code in...
catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed 768340b and pushed to 9.2.x. Thanks!

wim leers’s picture

Title: Simplify code in DrupalSqlBase » Simplify code in DrupalSqlBaseTest

Status: Fixed » Closed (fixed)

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