[#857] Aligned assertion and helper names with the documented naming rules.

[AlexSkrypnyk]

Closes #857

Summary

tests/phpunit/src/TraitMethodNamingTest.php gains 8 tests for the shape of an assertion name and the verb of a helper name, and 65 trait methods are renamed (58 public, 7 protected): the 59 the tests flag, plus 6 that follow the same documented rules inside their own trait. A context that calls or overrides cookieAssertWithNameNotExists() or stateAssertHasValue() now uses cookieAssertNotExistsWithName() or stateAssertValueEquals().

TraitMethodNamingTest read the trait prefix, the No and DoesNot negation words, the Is copula, Normalize spelling and the Find, Get and Load return types, and nothing else about a name's shape. So cookieAssertWithNameNotExists() put a qualifier ahead of Not, stateAssertHasValue() used Has to compare a value, metatagAssertRobotsIncludes() said Includes for Contains, cookieExists() asserted without Assert, commandAssertInteger() carried Assert while throwing \RuntimeException, and messageSelector() had no verb. All of them passed ahoy test-unit.

After the merge, ahoy test-unit runs 15 naming tests instead of 7 and CONTRIBUTING.md states each rule they read. MIGRATION.md maps 56 of the renames to their new names, and the other 9 are public names that exist only on 4.x (the latest tag is 3.14.3), so there's nothing to migrate. No Gherkin step text, step parameter name or method body changed, so no .feature file needs an edit, and assertion names with no predicate at all, such as tableAssertColumns(), keep theirs.

Before / After

What TraitMethodNamingTest reads
BEFORE: 7 tests                       AFTER: 15 tests
┌───────────────────────────────┐     ┌──────────────────────────────────────────────┐
│ trait prefix                  │     │ the 7 on the left, plus 8 that read:         │
│ No / DoesNot negation words   │     │ - Assert opens an assertion name             │
│ Is copula                     │     │ - Assert is followed by what it asserts      │
│ Normalize spelling            │     │ - Assert fails with an assertion exception   │
│ Find / Get / Load types       │ ──▶ │ - a qualifier follows the predicate          │
│                               │     │ - a negative step mirrors its positive       │
│ not read: Assert shape,       │     │ - Has names what a subject holds             │
│ qualifier, Has, Includes,     │     │ - Exists / Contains, not Present / Includes  │
│ Present, helper verbs         │     │ - a published helper carries a verb          │
└───────────────────────────────┘     └──────────────────────────────────────────────┘

1 example per rule, from the 65 renamed methods
┌──────────────────────────────────┬─────────────────────────────────┬─────────────────────────────────┐
│ rule (names renamed)             │ BEFORE                          │ AFTER                           │
├──────────────────────────────────┼─────────────────────────────────┼─────────────────────────────────┤
│ qualifier follows predicate (30) │ cookieAssertWithNameNotExists() │ cookieAssertNotExistsWithName() │
│ Has names what is held (14)      │ stateAssertHasValue()           │ stateAssertValueEquals()        │
│ Contains / Exists spelling (4)   │ metatagAssertRobotsIncludes()   │ metatagAssertRobotsContains()   │
│ Assert only on assertions (10)   │ commandAssertInteger()          │ commandParseInteger()           │
│ helpers carry a verb (7)         │ messageSelector()               │ messageGetSelector()            │
└──────────────────────────────────┴─────────────────────────────────┴─────────────────────────────────┘

Changes

Renames (src/) - 65 methods across 20 trait files, with every call inside the traits updated. The 5 groups below sum to 65.

• A qualifier follows the predicate (30). CookieTrait has 12 steps (cookieAssertWithNameExists() is now cookieAssertExistsWithName(), cookieAssertWithNameNotExists() is now cookieAssertNotExistsWithName(), and so on), LinkTrait has 6 (linkAssertTextWithHrefExists() is now linkAssertExistsWithHref(), linkAssertWithTitleExists() is now linkAssertExistsWithTitle()), ElementTrait has 4 attribute steps (elementAssertAttributeWithValueExists() is now elementAssertExistsWithAttributeValue()), MetatagTrait has 2 (metatagAssertWithAttributesNotExists() is now metatagAssertNotExistsWithAttributes()), and TableTrait has 5 (tableAssertTextInRow() is now tableAssertRowContains(), tableAssertLinkNotInRow() is now tableAssertLinkNotExistsInRow(), tableAssertMultipleTextsInRow() is now tableAssertRowContainsMultiple()). The 30th is the email step in the next bullet.
• The email case is different. emailAssertMessageSentToAddressWithContentNotContaining() negates its content check rather than the send, so Not can't move into the predicate slot without changing what it asserts, and emailAssertMessageNotSentToAddressWithContentContaining() already asserts the other thing. It becomes emailAssertMessageSentToAddressNotContains(), with the message sent to the address as the subject. It still asserts that an email went to the address and that no collected body contains the text.
• Has names something the subject holds (14). stateAssertHasValue() is now stateAssertValueEquals(), fileAssertUnmanagedHasContent() and fileAssertUnmanagedNotHasContent() are now fileAssertUnmanagedContains() and fileAssertUnmanagedNotContains(), the 4 elementAssert(Not)HasCssProperty*() steps are now elementAssertCssPropertyEquals(), elementAssertCssPropertyContains(), elementAssertCssPropertyNotEquals() and elementAssertCssPropertyNotContains(), and fieldAssertColorFieldHasValue() is now fieldAssertColorFieldEquals(). The 4 PathTrait query parameter assertions are now pathAssertUrlParameterExists(), pathAssertUrlParameterEquals(), pathAssertUrlParameterNotExists() and pathAssertUrlParameterNotEquals(), and watchdogAssertNotHasErrors() and javascriptAssertNotHasErrors() are now watchdogAssertErrorsNotExist() and javascriptAssertErrorsNotExist(). userAssertHasRoles(), elementAssertHasKeyboardFocus() and elementAssertHasVisibleFocusOutline() keep Has because the subject holds something.
• Contains and Exists, not Includes and Present (4). metatagAssertRobotsIncludes() and metatagAssertRobotsNotIncludes() are now metatagAssertRobotsContains() and metatagAssertRobotsNotContains(), and metatagAssertMetaSetPresent() is now metatagAssertMetaSetExists(). The protected fileDownloadAssertLinkPresent() is now fileDownloadGetLink(), because it returns the link and throws on a miss, which makes it a Get lookup. The step text still reads the meta robots should include :directive.
• Only an assertion is named Assert (10). cookieExists() and cookieNotExists() are now cookieAssertExists() and cookieAssertNotExists(), the protected configCompareEquals() and configCompareContains() are now configAssertEquals() and configAssertContains(), and messageAssert() and messageAssertNot() are now messageAssertExistsOfType() and messageAssertNotExistsOfType(). The 4 protected checks that throw \RuntimeException take the verb for what they do: commandAssertHasRun() is now commandRequireRun(), commandAssertNumeric() and commandAssertInteger() are now commandParseNumeric() and commandParseInteger(), and emailAssertLinkNumber() is now emailParseLinkNumber().
• Published helpers carry a verb (7). authLoggedIn() is now authIsLoggedIn(), dateNow() is now dateGetNow(), drushDriver() is now drushGetDriver(), messageSelector() is now messageGetSelector(), metatagOpenGraphRequired() and metatagTwitterCardRequired() are now metatagGetRequiredOpenGraphTags() and metatagGetRequiredTwitterCardTags(), and webformTemplates() is now webformLoadTemplates(). Docblock summaries that opened with Assert, Convert or Resolve for a renamed method now open with its new verb.

Naming test (tests/phpunit/src/TraitMethodNamingTest.php) - 8 new tests, each with its own data provider, read every trait under src/Steps and src/Helper.

• testAssertionsOpenWithAssert, testAssertionsNameWhatTheyAssert and testAssertionsFailWithAssertionException hold the <prefix>Assert<Subject><Predicate> shape. A Then step, or a helper whose docblock opens with Assert, must carry Assert right after the prefix, Assert must be followed by what's asserted, and an Assert method whose body throws must throw at least 1 of AssertionException, ElementNotFoundException or ExpectationException.
• testQualifiersFollowPredicate flags a word from QUALIFIERS (With, In, Of and the rest) that sits before a word from PREDICATES or directly after Not.
• testHasNamesWhatSubjectHolds flags Has followed by Content, Value or Text, or by a With or Containing value qualifier.
• testPredicatesSpelledExistsAndContains flags Absent, Include, Includes, Including, Missing, Presence and Present in an assertion name.
• testNegativeMirrorsPositive pairs every should not step with its should twin in the same trait and fails a pair whose method names differ by anything but Not.
• testHelpersCarryVerb reads the words of every public helper name against the VERBS list. A helper built on a verb the toolbox hasn't used yet adds that verb to the list in the same change.
• New helpers: isRegistered() tells a Behat step, transform or hook from a helper, docblockSummary() reads the first docblock line, thrownExceptionNames() lists the exception classes a method body throws, and insertsNot() checks that a negative name is its positive with Not added.
• Review still holds 3 things no test reads: a Has standing in for existence (the watchdog and javascript error checks), the PathTrait existence pair and tableAssertRowContainsMultiple(), which are renamed to match the other assertions in their own trait, and metatagGetRequiredOpenGraphTags(), because Open in the old metatagOpenGraphRequired() counted as a verb to the check.

Docs - CONTRIBUTING.md states qualifiers last, the Has rule, the Includes and Present ban and that only an assertion is named Assert, adds a "Helpers carry a verb" subsection and a metatagAssertWithAttributesNotExists() row in the negation table, and says what the test checks and what review holds. MIGRATION.md gets 4 new subsections under "One shape per naming idea" (a qualifier follows the predicate, Has names something the subject holds, Contains and Exists, only an assertion is named Assert) and extra rows in the override-point and lookup tables. Its 5 negation rows for watchdogAssertNoErrors(), javascriptAssertNoErrors(), fileAssertUnmanagedHasNoContent(), pathAssertUrlHasNoParameter() and pathAssertUrlHasNoParameterWithValue(), and the 2 rows that named authLoggedIn(), point at the final names. HELPERS.md and STEPS.md are rebuilt by ahoy update-docs, and STEPS.md changes only where DateTrait's docblock names dateGetNow(). docs/architecture/class-context.puml, class-context.svg and README.md name authIsLoggedIn() and pathAssertUrlParameterExists().

Callers - tests/behat/bootstrap/FeatureContext.php overrides dateGetNow() to pin the clock, and DateTraitTest, TableTraitTest and EntityLifecycleTraitTest call the new names.

Not changed here

• About 13 verbless protected helpers, such as fieldCurrentPath() and xmlDirectChildElements(), because testHelpersCarryVerb reads published helpers only.
• Assertion names with no predicate at all, such as tableAssertColumns(), regionAssertElementText(), fileDownloadAssertFileName() and metatagAssertOpenGraphTags().
• javascriptSupportAvailable() and queryAssertModuleEnabled(), which the issue also lists, no longer exist on 4.x, so there's nothing left to rename.

Testing

• Each rule's test was added before its renames. 7 of the 8 new tests failed on exactly the old names listed above and pass now. testNegativeMirrorsPositive passes on both sides, since it guards the 90 should and should not step pairs against a rename that moves only 1 half.
• ahoy test-unit passes on the rebased branch: 4086 tests, 5658 assertions, including the 15 TraitMethodNamingTest tests.
• ahoy lint passes (PHPCS, PHPStan, Rector, gherkinlint and the architecture checks), and so does ahoy lint-docs, both on the rebased branch.
• ahoy test-bdd passes locally on the full suite (1197 scenarios, 5057 steps), and all 16 CI test legs pass on the rebased branch: PHP 8.3 to 8.5, Drupal 11 and 12, Behat 3 and 4, including the 2 chrome_headless legs.
• Codecov reports every modified coverable line covered.

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 31 billable files and costs up to $7.75.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 14 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used all 3 included reviews currently available. Your 75 included PR review attempts over the past 7 days set your current allowance at 3 reviews per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Team

Run ID: b6e7909b-a610-43ae-9919-fab594a4e557

📥 Commits

Reviewing files that changed from the base of the PR and between f37dd87 and 74e61cb.

⛔ Files ignored due to path filters (1)

• docs/architecture/class-context.svg is excluded by !**/*.svg

📒 Files selected for processing (31)

• CONTRIBUTING.md
• HELPERS.md
• MIGRATION.md
• STEPS.md
• docs/architecture/README.md
• docs/architecture/class-context.puml
• src/Helper/Drupal/AuthTrait.php
• src/Steps/Drupal/ConfigTrait.php
• src/Steps/Drupal/DrushTrait.php
• src/Steps/Drupal/EmailTrait.php
• src/Steps/Drupal/FileTrait.php
• src/Steps/Drupal/StateTrait.php
• src/Steps/Drupal/WatchdogTrait.php
• src/Steps/Drupal/WebformTrait.php
• src/Steps/Web/CommandTrait.php
• src/Steps/Web/CookieTrait.php
• src/Steps/Web/DateTrait.php
• src/Steps/Web/ElementTrait.php
• src/Steps/Web/FieldTrait.php
• src/Steps/Web/FileDownloadTrait.php
• src/Steps/Web/JavascriptTrait.php
• src/Steps/Web/LinkTrait.php
• src/Steps/Web/MessageTrait.php
• src/Steps/Web/MetatagTrait.php
• src/Steps/Web/PathTrait.php
• src/Steps/Web/TableTrait.php
• tests/behat/bootstrap/FeatureContext.php
• tests/phpunit/src/TraitMethodNamingTest.php
• tests/phpunit/src/Unit/Helper/Drupal/EntityLifecycleTraitTest.php
• tests/phpunit/src/Unit/Steps/Web/DateTraitTest.php
• tests/phpunit/src/Unit/Steps/Web/TableTraitTest.php

• Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.

---

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.96%. Comparing base (f37dd87) to head (74e61cb).

Additional details and impacted files

@@           Coverage Diff           @@
##              4.x     #895   +/-   ##
=======================================
  Coverage   95.96%   95.96%           
=======================================
  Files         164      164           
  Lines        8395     8395           
=======================================
  Hits         8056     8056           
  Misses        339      339           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.