Skip to content

[3.0][Testing] Add an integration test suite that runs against a real forum - #9345

Open
albertlast wants to merge 4 commits into
SimpleMachines:release-3.0from
albertlast:tests/integration
Open

[3.0][Testing] Add an integration test suite that runs against a real forum#9345
albertlast wants to merge 4 commits into
SimpleMachines:release-3.0from
albertlast:tests/integration

Conversation

@albertlast

@albertlast albertlast commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Description

The unit suite from #9326 is deliberately database-free, and its own bootstrap
says what is missing: anything reaching Config::$modSettings, User::$me or
Db::$db "belongs in an integration suite running against a real install".
There was not one, so most of the forum had no automated proof of anything.

This adds it. tests/Integration/ is a second PHPUnit testsuite that runs
against a forum installed by #9344, on either engine:

.docker/test.sh                     # both engines
.docker/test.sh --engine postgresql

composer test still runs everything. When there is no forum to talk to the
integration tests skip rather than fail
, so it stays useful on a machine with
no Docker — 108 unit tests pass, 20 integration tests skip, exit 0.

What the base class provides

IntegrationTestCase gives each test:

  • a transaction, rolled back in tearDown(), so tests need not order themselves
    around each other. $modSettings is restored separately, because the rollback
    returns the table and not the static array;
  • actingAs($id) / adminId(), via User::setMe() — the same seam
    Login2::DoLogin() uses once it has checked the password;
  • hook($name, $function) with permanent: false, so it lives in $modSettings
    and disappears with the rollback;
  • assertNoErrorsLogged(), usually the point of the test. SMF records most of
    what goes wrong in log_errors rather than showing it, so a page that returned
    the right thing while quietly logging an undefined index has still regressed;
  • queryRow() / rawSetting(), which read past $modSettings and its cache.

Two things the rollback does not cover, both documented in the class: DDL, since
MySQL commits implicitly on CREATE/ALTER/DROP; and anything happening in
another process, such as a request made over HTTP.

The three tests
  • HarnessTest checks the harness itself — that the rollback really happens
    (one test writes, the next asserts it is gone), that $modSettings is
    restored, and that assertNoErrorsLogged() is capable of failing. A suite
    whose isolation quietly broke would not fail; it would start passing things it
    should not.
  • ModSettingsTest pins the counter regression fixed in [3.0] Makes the settings counters increment on PostgreSQL too #9340.
    updateModSettings($x, true) emitted SET value = value + 1 against
    settings.value, a text column.
  • SchemaTest compares Sources/Db/Schema/v3_0/ against the live database
    in both directions — declared tables and columns that are missing, and
    columns present that nothing declares. This is the drift AGENTS.md warns about,
    where a query naming a removed column fails at runtime only, and inside a
    background task retries forever.
Why both engines, concretely

Not thoroughness for its own sake. I reverted #9340's fix and re-ran:

MySQL PostgreSQL
with the fix 9 pass 9 pass
fix reverted 9 pass 5 fail

MySQL coerces text to a number and hides the bug completely. A suite that only
ever saw one engine would have proved nothing here. Worth knowing too: on
PostgreSQL a failed query poisons the rest of the transaction, so one swallowed
error turns every later query in that test into falsequeryRow() reports
that as a readable assertion failure rather than a TypeError about argument #1.

Notes
  • tests/bootstrap.php gains one line registering SMF\Tests\ for autoloading.
    The unit tests are each self-contained so nothing needed it before; anything
    sharing a base class does. Kept beside the two setPsr4() calls already there
    rather than adding an autoload-dev section only the suite would use.
  • composer.json gains test-unit and test-integration alongside test.
  • AGENTS.md is updated: it previously told contributors that Db::$db was
    simply out of scope.
Verified
  • .docker/test.sh — 128 tests, 196 assertions, green on both engines.
  • With Settings.php removed — 108 pass, 20 skip, exit 0.
  • Regression proven in both directions, as above.
  • check-signed-off.php, check-smf-index.php, check-smf-license.php,
    check-smf-languages.php and phplint all pass; shellcheck clean on all
    five .docker scripts.
Two commits that are not about testing
  • Lets the section comment fixer place its own banners — I wrote the section
    banners by hand with the wrong asterisk count, so SMF/section_comments did not
    recognise them and added its own alongside. AGENTS.md already says not to
    hand-write these; this takes what the fixer produces.
  • Removes the trailing tabs from a blank line in PM search — one character in
    Sources/PersonalMessage/Search.php, which I have otherwise not touched.
    c344b5c23 left a line holding nothing but three tabs. It has not turned CI red
    so far because the style workflow normally only checks the files a PR changes;
    it checks everything when composer.lock is in the diff, which this PR's
    does. Any other branch touching a dependency will hit the same thing, so it
    seemed better to fix than to work around. Happy to split it out if preferred.

Merge order

Merge #9326 and #9344 before this one. Both are contained in this branch, so the
diff shown here is theirs as well as its own; once they land and this is rebased on
release-3.0, what is left is tests/Integration/ and its bootstrap.

Issues References (Fixes|Related|Closes)

  1. Depends on [3.0][Testing] Install the forum from the command line #9344 — needs .docker/install-forum.sh to have a forum to test.
  2. Depends on [3.0][Testing] Add a PHPUnit suite for the parts that need no database #9326 — builds on the PHPUnit suite and its bootstrap.
  3. Related to [3.0][Testing] Add a Docker development environment for MySQL and PostgreSQL #9317, [3.0] Makes the settings counters increment on PostgreSQL too #9340.

albertlast and others added 4 commits August 29, 2026 00:03
The installer tells you to delete it and cannot do it itself: the ?delete
link it offers is a GET, and command line arguments only ever reach
$_POST, so nothing on the CLI path ever gets there.

Leaving it behind is not cosmetic. Settings.php redirects every request
back into the installer while the file exists, so the forum the script
just built is unreachable, and SMF puts a "MAJOR SECURITY RISK: you have
not removed install.php" box on every page it shows an administrator -
which also lands in front of anything else a test or a person is trying
to read on that page.

Deleting it is safe for a reinstall because install_one() calls reset.sh
first, and reset.sh clears Settings.php and then blocks until the
entrypoint has staged a fresh copy. Adds a check in front of the two
installer passes to say so out loud when it has not: without one, php
reports "Could not open input file: install.php", which reads like a
broken script rather than a stack that was never made installable.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Two forums side by side, each with its own administrator, and a password
chosen at install time is a combination that ends in hand written SQL
sooner or later - which is a poor way to answer a question as ordinary as
"is this the password?".

user.sh answers it. list shows the accounts, check says whether SMF would
accept a password and exits 0 or 1 so it can be used in a conditional,
and reset sets a new one. --engine reads the settings use-engine.sh saved
for the other engine, so the forum that is not currently live can be
looked at without switching to it and back.

Two details that stop it being a thin wrapper around an UPDATE:

  - The hashing goes through Security::hashPassword() rather than being
    written here, so what lands in the table is by construction what
    Login2 reads back out. A script that hashes passwords its own way is
    a script that eventually disagrees with the forum.
  - reset clears passwd_flood too. SMF locks an account out for a while
    after enough wrong guesses, and a new password behind a live lockout
    behaves exactly like a password that did not take.

check also points out an account that is not activated, which fails to
log in with an entirely correct password.

The password is passed to the container through the environment rather
than in the argument list, which anything able to read the process table
can see. Also completes the file list in the README, which still only
described the image and had none of the scripts in it.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
The unit suite is deliberately database-free, and says so: its bootstrap
notes that anything reaching Config::$modSettings, User::$me or Db::$db
"belongs in an integration suite running against a real install". There
was not one, so most of the forum had no automated proof of anything.

Adds tests/Integration/ as a second PHPUnit testsuite, and .docker/test.sh
to run it on one engine or both. composer test still runs everything;
when there is no forum to talk to the integration tests skip rather than
fail, so it stays useful without Docker.

IntegrationTestCase gives each test a transaction that is rolled back
afterwards, actingAs()/adminId() via User::setMe(), hook() registration
that lives only in $modSettings, and assertNoErrorsLogged() - which is
usually the point of the test, because SMF records most of what goes
wrong in log_errors rather than showing it.

Three tests to start:

  HarnessTest    checks the harness itself, including that the rollback
                 really happens and that assertNoErrorsLogged can fail
  ModSettingsTest  the counter regression: updateModSettings($x, true)
                 emitted SET value = value + 1 against a text column
  SchemaTest     compares Sources/Db/Schema/v3_0/ against the database in
                 both directions, which is the drift AGENTS.md warns only
                 ever shows up at runtime

ModSettingsTest is why running both engines matters rather than being
tidy: with the fix reverted it still passes on MySQL, which coerces text
to a number, and fails only on PostgreSQL, which refuses. Verified in
both directions before committing.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
The banners in the new test classes were written by hand with the wrong
number of asterisks, so SMF/section_comments did not recognise them and
inserted its own alongside, leaving IntegrationTestCase with two
"Internal properties" headings and two "Internal methods" ones.

AGENTS.md says not to hand-write these. Removes them and takes what the
fixer produces, along with the single_quote, native_function_invocation
and no_unused_imports changes it wanted in the same pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Installer Localization Language & internationalization

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants