Skip to content

[3.0][Testing] Add HTTP smoke tests that drive a running forum - #9347

Open
albertlast wants to merge 8 commits into
SimpleMachines:release-3.0from
albertlast:tests/http
Open

[3.0][Testing] Add HTTP smoke tests that drive a running forum#9347
albertlast wants to merge 8 commits into
SimpleMachines:release-3.0from
albertlast:tests/http

Conversation

@albertlast

@albertlast albertlast commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

Description

The integration suite from #9345 reaches the database, but never a page.
Everything between a request arriving and HTML coming back — the session, the
cookies, the theme, the templates, the permission checks — had no automated
coverage at all, and that is where the failures people actually report live.

tests/Integration/Http/ fixes that by driving a running forum over the wire.

.docker/test.sh                  # both engines, everything
.docker/test.sh --filter Posting

Requests have to be real ones: obExit(), redirectexit() and fatal*() all
end in exit, and Db::$db, ActionTrait::$obj and Theme::$loaded cannot be
reset, so a test process can carry out exactly one request in itself and no more.
tests/Support/HttpClient.php is a small browser built on the curl extension
the forum already requires
, so this costs no new dependency and no Node.

The tests
  • GuestPagesTest — a sweep of thirteen pages a visitor can reach, each
    asserting a real forum page came back (a fatal error in SMF is a normal page
    with an apology on it, so the status alone proves little) and that nothing was
    logged. Plus the RSS feed, a 404 for an unknown action, and registration.
  • LoginTest — signing in, the cookie being issued, a wrong password being
    refused, a post with no session check being refused, and signing out.
    Worth doing over HTTP precisely because User::setMe() skips all of it.
  • PostingTest — starting a topic and replying to it, then checking both are
    on the page and in the database; and that a guest cannot post.

Every one ends in assertNoErrorsLogged(). That is the point: SMF records most
of what goes wrong in log_errors rather than showing it, so a page can return a
flawless 200 while logging an undefined index on every hit.

Four things that made these harder than expected

Each is commented where it bites rather than worked around silently, because each
cost a debugging session and none of them are obvious from the symptom:

  1. The first request of a new session regenerates it. SMF sets a guest login
    cookie, and Cookie::setLoginCookie() throws the session away when that value
    changes — so a security token minted on the very first page a visitor sees can
    never be validated. It presents as a 403 "Token verification failed", which
    points at the token rather than at the session underneath it.
  2. Only the button that was clicked gets submitted. The posting form offers
    both preview and post; sending the pair means preview wins, the post is
    never made, and the response is a perfectly ordinary 200 with no topic behind
    it. formFields() therefore omits buttons and callers name the one they press.
  3. Flood control will hit you. Security::spamProtection() allows a moderator
    one login or post every two seconds per IP, and tests are far faster than
    people. submitForm() waits it out once, which is the difference between a
    suite people trust and one that fails now and then for no reproducible reason.
  4. curl only writes cookies that carry an expiry to its jar file. A handle
    opened per request loses the session cookie every time, so every request
    arrives as a new visitor — pages still render, but every POST is rejected.
Transactions

IntegrationTestCase gains usesTransaction(), and the HTTP tests return false
from it. They have to: the request runs in the web server's process on its own
connection, so nothing here is visible to it and nothing it does can be rolled
back — and on MySQL's default REPEATABLE READ an open transaction here would keep
reading the snapshot it took before the request, quietly making
assertNoErrorsLogged() incapable of ever failing. PostingTest removes what it
creates through SMF's own Topic::remove() instead.

One installer fix

install-forum.sh now removes install.php when it finishes. The installer says
to do this and cannot do it itself — ?delete is a GET and command line arguments
only reach $_POST — and leaving it puts a "MAJOR SECURITY RISK: you have not
removed install.php" banner across the forum. The new tests found it.

Credentials

The signing-in tests need to know the administrator. They default to what
install-forum.sh creates and can be pointed elsewhere with SMF_ADMIN_USER and
SMF_ADMIN_PASS. A password the suite does not know makes them skip with a
message saying so
, rather than fail — that is a misconfigured forum, not a
regression. Verified both ways.

Verified
  • 151 tests, 336 assertions, green on both engines, from the tree in this
    branch, with each forum's own administrator password.
  • The flakiness is genuinely fixed: repeated full runs on PostgreSQL, and repeated
    PostingTest runs, all clean.
  • check-signed-off.php, check-smf-index.php and check-smf-license.php pass;
    php-cs-fixer is clean under PHP 8.5; shellcheck clean on every .docker script.

Merge order

Merge #9345 before this one, and the PRs it names before that. This branch contains
the whole chain, so the diff shown here is mostly theirs; once #9345 lands and this is
rebased on release-3.0, what is left is tests/Integration/Http/.

Issues References (Fixes|Related|Closes)

  1. Depends on [3.0][Testing] Add an integration test suite that runs against a real forum #9345 — extends IntegrationTestCase.
  2. Depends on [3.0][Testing] Install the forum from the command line #9344install-forum.sh provides the forum, and is fixed here.
  3. Related to [3.0][Testing] Add a Docker development environment for MySQL and PostgreSQL #9317.

albertlast and others added 8 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>
The integration suite reaches the database, but not a page. Everything
between a request arriving and HTML coming back - the session, the
cookies, the theme, the templates, the permission checks - had no
automated coverage at all, and that is where the failures people actually
report live.

Requests have to be real ones: obExit(), redirectexit() and fatal*() all
end in exit, and Db::$db, ActionTrait::$obj and Theme::$loaded cannot be
reset, so a test process can carry out one request in itself and no more.
tests/Support/HttpClient.php is a small browser built on the curl
extension the forum already requires, so this costs no new dependency.

Three files to start: a sweep of the pages a guest can reach, the login
journey, and starting a topic and replying to it. Every one of them ends
in assertNoErrorsLogged(), which is the point - SMF records most of what
goes wrong in log_errors rather than showing it, so a page can return a
flawless 200 while logging an undefined index on every hit.

Four things about SMF made these harder to write than expected, and each
is commented where it bites rather than worked around silently:

  - The first request of a new session regenerates it, so a security
    token minted on the very first page a visitor sees can never be
    validated. It looks like a broken token, not a replaced session.
  - Only the button that was clicked gets submitted. The posting form
    offers "preview" and "post"; sending both means preview wins and the
    post is never made, with an ordinary 200 to show for it.
  - Security::spamProtection() allows one login or post every two seconds
    per IP, and tests are much faster than people, so submitForm() waits
    it out once instead of failing at random.
  - curl only writes cookies with an expiry to its jar file, so a handle
    opened per request loses the session every time.

HTTP tests cannot be wrapped in a transaction - the request runs in the
web server's process on its own connection, and on MySQL's REPEATABLE
READ an open transaction here would never see what it wrote, quietly
making assertNoErrorsLogged() incapable of failing. IntegrationTestCase
gains usesTransaction() so they can opt out, and PostingTest removes what
it creates through Topic::remove().

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
The suite ships the machinery but not the instructions for using it, and
three things are not discoverable from reading it: which of the three
suites a new test belongs in, who the request is actually made as, and
where the endpoint and field names come from.

The second is the one that misleads. HttpTestCase inherits actingAs()
from IntegrationTestCase, where it repoints User::$me in the PHPUnit
process - but the request is handled by Apache in another process, which
knows only the cookie. Calling it in an HTTP test changes nothing and
leaves the assertions describing a guest, confidently. The identity of a
request here is the cookie jar and nothing else.

Field names are the opposite problem: they look like something to look
up, and are not. submit() scrapes the form the way a browser does, which
is what carries the session check and the security token - both named
differently for every session, so a hand built POST body gets a 403 it
cannot fix. The worked example prints them to make that concrete.

Also notes that install.php left in the board root puts an errorbox on
every page an administrator sees, which fails assertLooksLikeAForumPage()
and crowds out whatever the test was looking at.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
The tests skip rather than fail when they cannot sign in, which says what
is wrong but not what to do about it. user.sh answers both halves: check
says whether the password the suite is using is the right one, and reset
puts a forum installed some other way back on the credentials it expects.

Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
The SMF/section_comments fixer puts the Public methods banner at the top of
the group, and the DataProvider attribute belongs to the method under it, not
above the banner. That one file was all the style check was failing on.

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