Problem/Motivation

The current tests have some strange formatting, e.g.

    $this->assertEqual($admin_status_plugin_definition['name'],
                       'Default Message',
                       'The default message admin_status plugin definition\'s' .
                         'name property is set.');

where strings are concatenated.

Proposed resolution

Look at core code and use that formatting.

Remaining tasks

  1. Review core code
  2. Create patch
  3. Review
  4. Commit

User interface changes

n/a

API changes

n/a

Data model changes

n/a

CommentFileSizeAuthor
#10 admin_status-clean_up_tests-2751625-10.patch2.75 KBkristen pol
#3 admin_status-clean_up_tests-2751625-3.patch2.85 KBkristen pol

Comments

Kristen Pol created an issue. See original summary.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

I've reviewed a bunch of core tests and it's common for the asserts to be in one line even if they are long like:

    $this->assertEquals($transform['expected_date'], $value, sprintf("Test \$date->setTimezone(new \\DateTimeZone(%s)): should be %s, found %s.", $transform['timezone'], $transform['expected_date'], $value));

or

    $this->assertEquals(array('expanded' => 1, 'has_children' => 0, 'provider' => array(array('module1', 'module2'), 'IN'), 'id' => array(1337, '<')), $parameters->conditions);

though sometimes they span lines like:

        self::assertEquals(
          $expected,
          self::readAttribute($actualClassOrObject, $actualAttributeName),
          $message,
          $delta,
          $maxDepth,
          $canonicalize,
          $ignoreCase
        );

or

        $this->assertEquals(
          'bar',
          $this->getObjectAttribute(new ClassWithNonPublicAttributes, 'privateParentAttribute')
        );

but in that case, each argument is on a separate line.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
StatusFileSize
new admin_status-clean_up_tests-2751625-3.patch2.85 KB

Here's the patch.

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Status: Active » Needs review
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Assigned: kristen pol » Unassigned
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Issue summary: View changes
kristindev’s picture

kristindev commented

Formatting cleaned up, looks good. RTBC

kristindev’s picture

kristindev commented
Status: Needs review » Reviewed & tested by the community
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented

Thanks!

kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
StatusFileSize
new admin_status-clean_up_tests-2751625-10.patch2.75 KB

I had to re-roll the patch due to some other patches that were committed.

  • Kristen Pol committed 01de412 on 8.x-1.x 
    Issue #2751625 by Kristen Pol, kristink2: Update tests to be consistent...
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Status: Reviewed & tested by the community » Fixed
kristen pol’s picture

kristen pol
she/her
Primary language English
Location Santa Cruz, CA, USA
commented
Issue summary: View changes

Status: Fixed » Closed (fixed)

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