Skip to content

03: First green CI - #3

Open
nikolaystrikhar wants to merge 9 commits into
mainfrom
03-ci-tests
Open

03: First green CI#3
nikolaystrikhar wants to merge 9 commits into
mainfrom
03-ci-tests

Conversation

@nikolaystrikhar

@nikolaystrikhar nikolaystrikhar commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Smoke test proving the harness, plus the PHP tests workflow.

Uses wp-browser's UopzFunctions instead of a local WithUopz trait.

@nikolaystrikhar nikolaystrikhar changed the title First green CI 03: First green CI Jul 31, 2026
Five defects from the PR review, all latent because tests/unit was empty:

- tests/unit/_bootstrap.php was missing while unit.suite.yml declares
  `bootstrap: _bootstrap.php`. Codeception resolves that against the suite
  path and throws ConfigurationException("Bootstrap file ... can't be
  loaded") before any test runs. The file is docblock-only: the global
  tests/_bootstrap.php already loads the autoloader.
- codeception.dist.yml had no `namespace`, so `codecept build` generated
  the actions trait as `_generated\UnitTesterActions` at the root, while
  UnitTester.php's trait use resolves relative to its own namespace. The
  actor could never be instantiated, and the generated file was not
  autoloadable under the Tests\Support PSR-4 mapping either.
- Dropped the root `actor: Tester`. The only suite declares its own actor,
  and combined with `actor_suffix: Tester` the key implied "TesterTester".
- Dropped `theme: twentytwentythree`. It was pinned against
  WP_VERSION=latest, and core rotates bundled default themes; nothing in
  the suite needs a specific theme.
- The multisite env now uses its own table prefix. Both envs share a
  database and WPLoader drops and recreates every table under its prefix
  on install, so a shared prefix makes concurrent env runs race.

Also adds tests/_data/.gitkeep, since codeception.dist.yml declares the
path but the directory was never created.

Verified: `codecept build` now emits
Nexcess\PluginAbsorber\Tests\Support\_generated, the actor class resolves
with the trait applied, the suite bootstrap loads, and the multisite env
merges to multisite=true / prefix=mstest_.
slic reads SSH_AUTH_SOCK unconditionally when preparing its docker-compose
env, and GitHub Actions runners don't have an ssh-agent running by default,
so every matrix leg failed at 'slic here' with 'SSH_AUTH_SOCK environment
variable is not set!'.
Comment thread tests/_support/Traits/WithUopz.php Outdated
Comment thread tests/_support/Traits/WithUopz.php Outdated
Comment thread .github/workflows/tests-php.yml Outdated
Base automatically changed from 02-codeception-harness to main July 31, 2026 15:04
Drops tests/_support/Traits/WithUopz.php. lucatume\WPBrowser\Traits\UopzFunctions
ships with wp-browser, is maintained by its author, undoes overrides through its
own @after, and exists as far back as the ^3.6.5 floor this library pins, so
there is no copy left to drift against the other plugin repos. Its
setFunctionReturn() also takes an explicit $execute flag rather than inferring
one from the value.

exit is never mocked. Neutralising exit lets a test keep running past the point
production would have halted, so a test that should fail can report as passing
and CI will not say otherwise. test_exit_can_be_neutralised is gone; redirect
branches are tested by stubbing the call immediately before exit and throwing
TestException from it, with a worked example in tests/README.md.

CI: narrow the token to contents:read, cancel superseded PR runs, add a 20 minute
timeout, upload tests/_output on failure, and run multisite even when singlesite
failed so one run reports both. The matrix is PHP 7.4 and 8.5 against WordPress
latest and nightly -- four legs, nightly non-blocking. Every leg now pins core
explicitly: the slic image bakes in a WordPress that varies by PHP version, so
the previous "latest" legs silently tested whatever core the image happened to
ship rather than latest.

composer.json maps Tests\Unit\ to tests/unit. The previous Tests\ => tests/
mapping resolved SmokeTest to tests/Unit/SmokeTest.php, which Composer skips from
the optimised classmap on a case-sensitive filesystem.

Plan and design docs updated so Tasks 8 and 13 do not reintroduce the trait.

Verified locally: slic run unit --env singlesite and --env multisite both report
OK (3 tests, 5 assertions), the PSR-4 warning is gone, and the multisite env
really installs a network (mstest_site and mstest_sitemeta present).

Claude-Session: https://claude.ai/code/session_01YWju62NPWe1dtp1fysao9Q
Codeception aborts with ConfigurationException("register_argc_argv must be
set to On") before it builds. slic's php.ini does not set the directive, so
the base image default applies and it differs by PHP version: the 7.4 legs
passed, both 8.5 legs failed at 'slic cc build'.

slic is checked out into the workspace and its containers/slic/php.ini is
bind-mounted as conf.d/zz-docker.ini, which loads after the main php.ini, so
appending the directive there overrides the image default.

Claude-Session: https://claude.ai/code/session_01YWju62NPWe1dtp1fysao9Q
The rule that exit is never mocked -- stub the call before it and throw
TestException -- shipped documented in three places and exercised by zero
tests. TestException had no callers at all. The smoke test exists to prove
the harness assumptions before library code leans on them, and it proved
two of them (WordPress loads, a function can be stubbed) while the newest
and least conventional one went unchecked: that an exception raised inside
a uopz-replaced function escapes it and is catchable by the test.

test_a_stub_can_throw_to_halt_a_code_path closes that. Verified the
mechanism independently of CI first, in a php:8.3-cli container with uopz
built from pecl: uopz_set_return( $fn, $closure, true ) with a throwing
closure propagates the exception out of the replaced function with its
message intact.

test_the_env_matches_its_table_prefix gives the multisite CI leg something
to prove. Both envs previously ran the identical three assertions, so a
multisite env that quietly failed to install a network -- or that picked up
the wrong tables -- still reported green. Asserting is_multisite() against
base_prefix covers the network and the per-env tablePrefix together.

tests/README.md: the worked example did not compile ($subject was never
defined) and stacked expectException, a try/catch and a re-throw to do one
job. It is now the catch-and-flag shape, with the reason spelled out --
catching without asserting the exception arrived turns "the code never
redirected" into a silent pass, which is the failure mode the section opens
by warning about.

Claude-Session: https://claude.ai/code/session_01YWju62NPWe1dtp1fysao9Q
Five things that were each fine while nothing moved.

slic was checked out at ref: main, so an upstream reorganisation could
change CI without a commit here -- and the register_argc_argv step reaches
directly into slic's file layout. Pinned to 2.3.0, which is the same commit
main points at today, so behaviour is unchanged.

That step also used a bare >> redirect, which creates the file if slic ever
moves or renames containers/slic/php.ini. The override would stop applying
silently and surface later as an unrelated-looking cc build failure on 8.5
only. It now fails loudly when the target is missing.

The multisite step ran on !cancelled(), whose stated intent was "run even
when singlesite failed". It also ran after a failed composer install or
build, stacking a second misleading failure on the real one. Now gated on
the build having succeeded.

continue-on-error keeps a red nightly leg from blocking the PR, but it also
makes a failing leg report as a pass in the checks list -- the only way to
notice was to open the run. The leg stays non-blocking; a failure now
leaves a warning annotation and a job-summary note where someone will see
it. Dropping continue-on-error entirely is a one-line change if a plain red
X is preferred; nothing is gated on it today, since the repository has no
branch protection or rulesets.

WP_VERSION=latest in .env.testing.slic was inert -- the workflow's own
comment says so, which is why the site-cli core update step exists -- and
on the nightly legs it claimed the wrong thing. Removed, with a note
pointing at what actually sets the version.

Also: workflow_dispatch, so a nightly leg can be re-run without pushing;
the checkout directory is no longer hardcoded; and the redundant CI=1 export
is gone, since Actions already sets it.

Claude-Session: https://claude.ai/code/session_01YWju62NPWe1dtp1fysao9Q
@nikolaystrikhar
nikolaystrikhar marked this pull request as draft July 31, 2026 16:13
The new smoke test failed on all four legs with

  [Error] Cannot access self:: when no class scope is active

which is the test doing its job on its first run. uopz executes a
replacement closure outside the test object, and my local check missed it
because calling a static closure directly from PHP does keep class scope --
only the uopz path does not.

Probed the actual boundary in a container running uopz. Inside a closure
installed with setFunctionReturn( $fn, $closure, true ):

  $this        fatal, "Using $this when not in object context"
  self::CONST  fatal, in static and non-static closures alike
  use ( ... )  works, including by reference
  arguments    arrive normally

So the message is resolved into a local and captured by value.

This matters well beyond the smoke test. The planned Resolver and
end-to-end tests stub deactivate_plugins() and wp_safe_redirect() with
closures that collect into $this->deactivations / $this->redirects and
throw with self::HALTED_AT_EXIT -- every one of those would have failed the
same way, in a later PR, with nothing to point at the cause. The working
shape is to bind a reference to the property, resolve constants into locals
and capture both:

  $deactivations = &$this->deactivations;
  $halt_message  = self::HALTED_AT_EXIT;

Verified through uopz that writes made through the captured reference do
land on the property. tests/README.md now documents the constraint with
both the broken and the working form.

Also drops a reference to WP_VERSION from a workflow comment, since the key
it named no longer exists.

Claude-Session: https://claude.ai/code/session_01YWju62NPWe1dtp1fysao9Q
@nikolaystrikhar
nikolaystrikhar requested a review from d4mation July 31, 2026 16:18
@nikolaystrikhar
nikolaystrikhar marked this pull request as ready for review July 31, 2026 16:18
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.

2 participants