Skip to content

Test isolation: shared base, Commands_Test isolation, real Services_Test (#14, #15) - #13

Merged
akshat009 merged 4 commits into
mainfrom
fix/test-isolation-and-services-test
Aug 29, 2026
Merged

Test isolation: shared base, Commands_Test isolation, real Services_Test (#14, #15)#13
akshat009 merged 4 commits into
mainfrom
fix/test-isolation-and-services-test

Conversation

@akshat009

Copy link
Copy Markdown
Owner

Re-audit #2 follow-ups.

#14 — static state was leaking between tests

  • tests/Unit/Plugin_TestCase (abstract, ships in every scaffold): tearDown() calls Services::reset() + Plugin::set_instance( null ). Every generated *_Test.php now extends Plugin_TestCase instead of PHPUnit's TestCase.
  • Commands_Test gets @runInSeparateProcess / @preserveGlobalState disabled — its define( 'WP_CLI', true ) can no longer persist into later tests. Schema_Test's front-end-gate test drops its markTestSkipped guard, so the Test isolation: shared base, Commands_Test isolation, real Services_Test (#14, #15) #13 fix is verified on every run again.

#15 — Services_Test didn't test the contract

Was assertTrue($ref->isStatic()) + ReflectionProperty into a private array (because set( object ) couldn't feed a \stdClass through a typed accessor). Now generated per-accessor from addService():

$a = $this->createMock( \Ns\Cache\Cache_Service::class );   // satisfies the return type
Services::set( 'cache', $a );
$this->assertSame( $a, Services::cache() );
$this->assertSame( Services::cache(), Services::cache() );     // memoisation
Services::reset();
// ... a fresh double comes back after reset()

Services_Test ships only when there's >= 1 accessor.

#11 / #12 revised (from the audit's own retractions)

  • templates/ phpcs: the blanket */templates/* exclude is justified (the audit retracted "drop it"), but it over-suppressed EscapeOutput on the HTML email template. Now, when a woo:email / woo:my-account module ships templates/, phpcs.xml adds <file>./templates</file> and excludes only PrefixAllGlobals for */templates/* and EscapeOutput for */templates/emails/plain/*. Gated on has_wc_template_overrides.
  • composer prepare-dist = composer install --no-dev --optimize-autoloader, so plugin-zip doesn't depend on reading the release docs.

Not in this PR

Verified: 69 generator + 13 engine tests; full/minimal scaffolds php -l clean, valid phpcs.xml both ways, every unit test extends Plugin_TestCase.

…l Services_Test (#14, #15)

#14 — static state was leaking between tests. New `tests/Unit/Plugin_TestCase`
(abstract, always shipped) resets `Services` and the `Plugin` singleton in
tearDown(); every generated `*_Test.php` now extends it instead of PHPUnit's
TestCase. `Commands_Test` gets `@runInSeparateProcess` / `@preserveGlobalState
disabled` so its `define( 'WP_CLI', true )` can't persist into the rest of the
run — `Schema_Test`'s front-end-gate test no longer has to `markTestSkipped`,
so the #13 fix is verified again.

#15 — `Services_Test` was a tautology + reflection into a private array, because
`set( object )` couldn't feed a `\stdClass` back through a typed accessor. Now
generated per-accessor from `addService()`: `createMock()` of the real class
(satisfies the return type), `Services::set()`, then assert the accessor hands
back that instance and the same instance twice, and that `reset()` clears it —
all through the public API. `Services_Test` only ships when there's >= 1
accessor.

Verified: 68 generator + 13 engine tests; full + minimal scaffolds php -l clean,
every unit test extends Plugin_TestCase, Services_Test present only with an
accessor.
…repare-dist (#11, #12 revised)

Re-audit #2 retracted "drop the */templates/* exclude" — the WC email/My-Account
overrides genuinely fail three sniffs (PrefixAllGlobals on core WC hook names +
loosely-named locals; EscapeOutput on plain-text bodies that use
wp_strip_all_tags). But the blanket path exclude also silenced EscapeOutput on
the HTML email template, where escaping does matter.

Now: when a woo:email / woo:my-account module ships a templates/ dir,
phpcs.xml adds `<file>./templates</file>` and excludes only:
  - WordPress.NamingConventions.PrefixAllGlobals for */templates/*
  - WordPress.Security.EscapeOutput for */templates/emails/plain/* only
Everything else in templates/ (docblocks, spacing, i18n, the HTML email's
escaping) is linted. Gated on a new has_wc_template_overrides flag; a scaffold
without those modules has no templates/ reference at all.

Also: `composer prepare-dist` = `composer install --no-dev --optimize-autoloader`,
so `npm run plugin-zip` doesn't depend on the user reading the release docs.
@akshat009
akshat009 merged commit b64cec7 into main Aug 29, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant