Skip to content

[3.0][Testing] Cover four more merged fixes in the unit suite - #9586

Open
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/tests-second-sweep
Open

[3.0][Testing] Cover four more merged fixes in the unit suite#9586
albertlast wants to merge 1 commit into
SimpleMachines:release-3.0from
albertlast:3.0/tests-second-sweep

Conversation

@albertlast

@albertlast albertlast commented Aug 30, 2026

Copy link
Copy Markdown
Collaborator

Note

This change was produced by an LLM. The tests, the commit message and this
description were all written by Claude (Anthropic), driven by @albertlast. It has
not yet had human code review.

The suite was run rather than only reasoned about, and every test below was also
run against the unfixed code it covers. Please review it as untrusted work.

Description

The second instalment of what #9326 was for, following #9511. Every bug fix merged
into release-3.0 since that sweep was looked at, including the backlog that landed
in one batch on the 29th and 30th, asking of each one whether the unit suite can
reach it and writing a test where it can.

Most of them cannot be reached: templates, JavaScript, SQL, or PHP that wants
Db::$db, User::$me, a session or a loaded theme. Four can.

Fix What is now covered
#9484 SpoofDetector::checkReservedName() on the list as the installer writes it, plus real line breaks, homographs, entities, and the reserveWord and reserveCase settings
#9409 MessageFormatter::formatMessage() given a \Stringable, including one whose value contains MessageFormat syntax
#9453 PageIndex keeping the verdict on an out-of-bounds start through __toString()
#9440, #9442 Avatar resolving a gallery avatar, on a forum at the root of its domain and on one in a subdirectory

33 new tests and 48 new assertions, in four new files. No existing test was
touched:

File Tests Assertions Covers
tests/Unit/SpoofDetectorTest.php 14 18 #9484
tests/Unit/MessageFormatterTest.php 6 6 #9409
tests/Unit/PageIndexTest.php 7 14 #9453
tests/Unit/AvatarTest.php 6 10 #9440, #9442

That takes the suite from 169 tests and 247 assertions to 202 tests and 295
assertions
, still under a second.

These are regression tests, not tests written to fit

Each set was run against the code as it was before its fix, by checking out the
single source file at the commit before the merge.

On the fixtures

AvatarTest looks for avatars/default.png and avatars/Oxygen/beagle.png, both
of which the repository ships. It passes id_member to the constructor, which is
the one argument that keeps it away from the attachments table, and it puts
Config::$boardurl and the three avatar settings back in tearDown().

SpoofDetectorTest covers checkReservedName() only. checkSimilarMemberName()
and checkSimilarGroupName() ask the members and membergroups tables what else
is out there and belong in the integration suite.

What is deliberately not here

The rest of the sweep. Named so the next one does not re-derive it: #9576, #9490,
#9485, #9483, #9477, #9465, #9464, #9461, #9460, #9459, #9435, #9434, #9422, #9414,
#9413, #9406, #9400 and #9380 all need a database; #9573, #9481, #9476 and #9412
need a request or a loaded theme; #9417 and #9315 are inside log(), which writes
a row; #9463 and #9348 are SQL that only one engine can disagree about; #9480 wants
a real image and GD; #9420 needs the BBCode parser, which needs both. #9402, #9418,
#9431, #9462, #9500, #9549 and #9558 are templates.

Table::populate() is the other half of #9484 and is not covered, because it
inserts rows.

Issues References (Fixes|Related|Closes)

Related to #9326, #9511, #9484, #9409, #9453, #9440, #9442.

A second sweep of the bug fixes now on release-3.0, in the same spirit as
SimpleMachines#9511: ask of each one whether the suite can reach it, and write a test where
it can. Everything merged since that sweep was looked at, along with the
backlog that landed in one batch on the 29th and 30th. Most of it is templates,
JavaScript, or PHP that wants Db::$db or User::$me. Four fixes do not.

SimpleMachines#9484 made SMF\Unicode\SpoofDetector::checkReservedName() split the admin's
list on the two characters backslash and n as well as on a real newline. The
installer writes the default list with the separators spelled out that way, so
splitting on newlines alone gave one long name nobody would type and every
reserved name was free to register.

SimpleMachines#9409 made SMF\Localization\MessageFormatter::formatMessage() flatten a
\Stringable argument to its string value. The class skips any argument that is
not already a string and hands the intl formatter only the scalar ones, so an
object argument reached neither and the member was shown the placeholder.

SimpleMachines#9453 made SMF\PageIndex remember, across __toString(), that the start value it
was handed was out of bounds. fixStart() records that as a side effect of
clamping, and __toString() called it again on a value already clamped, so the
verdict was always thrown away: page 1 came out as plain text rather than a
link, with a "next page" link beside it.

SimpleMachines#9440 and SimpleMachines#9442 both concern a gallery avatar, which is stored as a path under
the avatars directory rather than as a URL. Read as a URL, it was worked back
to a file from the URL's path, which lands outside the avatar directories; and
on a forum at the root of its domain that path is null, so stripping the board
URL off it threw a TypeError on every page the member appeared on.

Each set was run against the code as it was before its fix, by checking out the
single source file at the commit before the merge:

- SpoofDetector.php before SimpleMachines#9484: one failure, the installer's list.
- MessageFormatter.php before SimpleMachines#9409: three failures, all the \Stringable cases.
  The plain string, the number and the no-placeholder message pass either side.
- PageIndex.php before SimpleMachines#9453: two failures. The four tests covering an ordinary
  start pass either side, which is what makes them the control.
- Avatar.php before SimpleMachines#9440: five of six fail, the root-of-domain cases with the
  TypeError and the subdirectory ones by falling through to default.png. With
  SimpleMachines#9440 but not SimpleMachines#9442, four still fail: every gallery avatar becomes the
  default image.

202 tests, 295 assertions, still under a second.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant