Make every test helper name say what it does at the call site, and fold the duplicate fixtures into their traits

#3619693 took the identical-body count to zero and #3619712 gave the noun-named helpers a verb. What was left is one complaint in several shapes: the name at the call site does not say what the thing does, and the meaning lives in a docblock the reader has to go and find.

Fixtures wearing a name a shared trait owns

36 of them. Seven were live: those classes use SingleBookingHoldTrait and redeclare one of its methods, so the trait's version silently never applied. One would have broken rather than misled: AreaVenueTrait::createBookableSlot(Venue, Configuration, Grade, Section) against SlotBookingPlacementTest::createBookableSlot(Venue, Grade, ?Configuration), different arity with two arguments swapped, called by the trait's own fixtures. And that class's createSeatedVenueAndSlot() built a venue, a grade, a section and places, and no slot at all.

Thirteen were a shared fixture written out again and are gone, including five copies of one tariff builder that turned out to be a single method once measured against each other rather than against the trait.

The rest keep their bodies, because they do not collapse: the venue-with-places family is 26 methods at a median pairwise similarity of 0.51 (9 of 325 pairs above 0.80), the mapped family 10 at 0.42 (1 of 55). One builder for either would take a capacity, a place count, a section count, a grade count, a mode and a label. It also matters that three of them measure query budgets, where a fixture shared with tests that measure nothing can move a number for a reason unrelated to the code.

Names that needed the signature to be understood

24 bare verbs with no object across 155 call sites: act(), reset(), take(), open(), build(), read(), hold(), check(). Nouns pretending to be calls: areas(), offers(), page(), basket(). Names their own return type contradicted: getQuantity() handing back a NodeElement, cacheablePage() a Response, getSlotState() the slot's offers. Adjectives standing in for entities: $open and $restricted holding a tariff, $booker holding a Response. And $autoResource, one line from $manualSettlementResource, where manual meant two different things.

26 helpers returned a value with no declared return type, so a call site could not tell what came back. All 26 already carried the type in a @return docblock, so it was promotion rather than judgement, and phpstan checked every one.

A transaction is not an order

The engine stores a transaction; an order is yoyaku_order's decoration of it. 24 helpers were named for the order while typed BookingTransactionInterface. The module boundary sorts them: a module that does not declare yoyaku_order has no order, so 18 say transaction now, including the core trait's reloadTransaction() and loadTransactionBookings(); the six in modules that do declare it keep the word. That frees the name where it belongs, so OrderTest::reloadOrder() is honest again and the wrapper that existed only because a core trait had taken the name is gone.

Two service ids that nobody had named

PaymentPolicyResolverTrait and VenueMapBuilderTrait. The second id was written out 38 times across 15 files, twice behind a private getter and the rest through the container by hand.

Found while reading

#3619712 shipped a rename that was wrong. Its verb sweep renamed PoolOpTest::session() to createSlotWithTariffs(), which creates no slot, returns a Session, and wore a name three other classes use for a slot fixture. Fixed here as getRequestSession().

One assertion in the suite could not fail: CheckoutLockTest::testNonBookingRunIsIgnored() ended in assertTrue(TRUE) while its docblock claimed the run was left alone. Fixed here rather than filed, because the test was reporting coverage it did not have. The run is now given a bystander order it does not name, and the test asserts that order is still pending immediately after the opening event: a subscriber reaching for the current or the newest transaction instead of the one its own run names passes every other test in this class, and locks the bystander here. The placement is the whole point, and it is where the first attempt went wrong: the opening and the closing are a round trip, as testClosingUnlocksTheOrder shows, so a wrong lookup locks the bystander on the opening and unlocks it again on the closing, and an assertion made after both events sees pending either way and catches nothing. An assertNull on the correlator pins the premise, so a fixture that started binding a transaction fails outright rather than quietly turning this into a second pass over the ordinary path. Proved by injecting that exact bug into CheckoutLockSubscriber::resolveOrder(), falling back to the newest transaction when the correlator returns none: with the assertion after both events the class passed 1/1, and with it after the opening the class failed on this method alone. Letting a scenario report every independent check it failed rather than only the first still wants its own issue.

Not a collision

resetBeforeScenario() is declared in 35 classes and all 35 are right: ScenarioRunTrait ships an empty body and its docblock asks a test to override it.

Not in scope

Any change to what the tests assert, with the single exception described above. Comments were added above asserts that needed them, and every touched class passes.

AI-Generated: Yes (Claude Code took the measurements above from the test suite, wrote the changes on the merge request and drafted this summary. I reviewed them before posting.)