Problem/Motivation

Reduce confusion for testers by cleaning up some user-facing and internal labels (minor housekeeping cleanup)

At the code level, most of the tests have somewhat arbitrary @group annotations.
This inconsistency is also seen sometimes in core - (cf. System/system, Shortcut/shortcut) but it's not deliberate, just a messy oversight.

the various labels assigned to 'rules' tests.

This is only really visible at the UI level when you try to use the site-based test suite runner ( /admin/config/development/testing ) but test groups can also be used as arguments to the CLI phpunit runner as well ( eg `phpunit --group 'Rules' ).

Proposed resolution

Re-label a few of the test 'groups' for consistent capitalization, plurals, and correct categories.
Based on core examples, and for consistency with the PHP object names, this looks like these should become CamelCase. "rules_conditions" -> "RulesCondition"

This will in some cases require thinking about useful names, and naming things is hard.

OR?

Should everything in the 'Rules' namespace just be grouped as 'Rules'?
...When in doubt, just ensure the @group is 'Rules'.

Patch will affect user-facing text in code comments and annotations only - so no functional changes should be expected, except in cases where someones CI system is already using the --group filter.

User interface changes

Interface seen at admin/config/development/testing will group the 'Rules' tests more sensibly.

Testers will be able to refer to these groups by label if running or choosing tests via CLI.
These changes only affect the developer experience for testing.

CommentFileSizeAuthor
Screen Shot 2017-09-29 at 3.52.29 PM.png31.93 KBdman

Comments

dman created an issue. See original summary.

dman’s picture

dman commented
Status: Active » Needs review

Approach:

Generally,
anything in the functional tests (BrowserTestBase) should probably be called RulesUi.
anything lower-case 'rules' needs to become title case 'Rules'
Where sensible, additional sub labels may make sense. The each of the RulesAction and RulesCondition and RulesEvent tests (now singular) are already in convenient directories, so keep that pattern,

Gotcha:

RulesLoggerChannelTest was in @group 'Logger' - it doesn't seem conventional to be adding our tests under some other modules namespace, so I set it back to 'Rules'.
There was a stray @group inside one of the test objects functions AnnotationProcessingTest->testCheckConfiguration(), serving no purpose. Removed it.

Before patch:

../../vendor/bin/phpunit --list-groups ../modules/contrib/rules/tests/src/

Available test group(s):
 - Logger
 - Rules
 - rules
 - rules_action
 - rules_actions
 - rules_conditions
 - rules_events
 - rules_ui

After patch:

../../vendor/bin/phpunit --list-groups ../modules/contrib/rules/tests/src/

Available test group(s):
 - Rules
 - RulesAction
 - RulesCondition
 - RulesEvent
 - RulesUi

Pull request posted at https://github.com/fago/rules/pull/491

jonathan1055’s picture

jonathan1055 commented
Title: Review test @group annotations for consistency » Rationalise @group annotations for consistency
Status: Needs review » Reviewed & tested by the community

This all looks good. I have tested the changes locally (via Github PR). The numbers are:

Group Tests Assertions Type(s)
Rules 133 437 kernel and unit
RulesCondition 30 167 unit
RulesUi 9 52 all functional and one kernel
RulesAction 92 218 unit
RulesEvent 17 60 kernel and unit
Total 281 934

This matches with Travis build output
OK (281 tests, 934 assertions)

fago’s picture

fago
Primary language German
Location Vienna
commented
Status: Reviewed & tested by the community » Fixed

That's a good improvement, thank you! Merged!

  • dman authored a1ef192 on 8.x-3.x 
    Issue #2912782 : Improve test @group annotations

Status: Fixed » Closed (fixed)

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