[3.0][Testing] Add unit tests for SMF\Lang and TimeInterval::localize() - #9582
Merged
Sesquipedalian merged 3 commits intoAug 29, 2026
Merged
Conversation
Previously, trying to load any language strings required the theme to be loaded, which in turn required the database to be loaded. Now we simply skip the theme stuff if the database is not available. Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Until the change this builds on, none of it was reachable from a test. Asking for a language string went load() -> addDirs() -> Theme::loadEssential(), and the Theme constructor's first act is a query, so a process with no connection died on "Typed static property SMF\Db\DatabaseApi::$db must not be accessed before initialization" thrown out of Theme.php - a message with nothing about languages in it, three calls below where the test was looking. Seven tests, all of which fail with that error against release-3.0 and pass on top of the parent commit: loading a file, defaulting to the forum language when there is no user, the directories addDirs() settles on when there is no theme to ask, a custom directory that is not there being ignored, listing the installed languages, loading a file because a string was asked for from it, and finding a string that exists only on disk. Worth being precise about what is new: censorText(), sentenceList(), numberFormat(), formatText(), tokenTxtReplace() and getLocaleFromLanguageName() never needed the database and could always have been tested. What was blocked is everything that has to find a file first. Lang::$dirs is read through reflection in two of these. It is private and has no accessor, but which directories end up in it is the whole behaviour under test, and every other assertion here can only see that a file was found somewhere. PHPUnit does not reset SMF's statics between tests, and each of these leaves a loaded language behind, so tearDown() puts Lang back to nothing loaded. The suite is green in random order as well as in file order. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
This file carried a note saying localize() belonged to an integration suite: every branch of it goes through Lang::getTxt(), which loads a language file, which wanted Theme::$current and therefore Db::$db. That is no longer true on top of the parent commit, so the note goes and the tests it stood in for arrive. They cover the other half of what SimpleMachines#9499 put right, which toParsable() above could only imply with the strings hard coded: each unit pluralised on its own value into a sentence, the unit order not depending on how the caller wrote the argument, and asking for the total number of days falling back to years, months and days when the interval has no total rather than producing a flat "0 days". Plus the two edges around an empty result - saying zero of the smallest unit asked for, and folding fractional seconds into the seconds. All five fail against release-3.0 with the database error the note described, and pass here. The thirteen tests already in the file are untouched. tearDown() puts Lang's statics back, for the same reason the Lang tests do. Signed-off-by: albertlast <mathiaspapealbert@hotmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
#9581 removes
SMF\Lang's dependency on the database. This is the coverage thatunlocks, on top of it.
Until that change, none of the file-reading half of
SMF\Langwas reachable from atest. Asking for a language string went
load()->addDirs()->Theme::loadEssential(), and theThemeconstructor's first act is a query, so aprocess with no connection died on
Typed static property SMF\Db\DatabaseApi::$db must not be accessed before initializationthrown out ofTheme.php- a message withnothing about languages in it, three calls below where the test was looking.
tests/Unit/LangTest.phpSeven tests, 15 assertions: loading a file, defaulting to the forum language when
there is no user, the directories
addDirs()settles on when there is no theme toask, a custom directory that is not there being ignored, listing the installed
languages, loading a file because a string was asked for from it, and finding a
string that exists only on disk.
Worth being precise about what is new, because it is narrower than it looks:
censorText(),sentenceList(),numberFormat(),formatText(),tokenTxtReplace()andgetLocaleFromLanguageName()never needed the database andcould always have been tested. What was blocked is everything that has to find a file
first.
Lang::$dirsis read through reflection in two of these. It is private and has noaccessor, but which directories end up in it is the whole behaviour under test, and
every other assertion can only see that a file was found somewhere.
tests/Unit/TimeIntervalTest.phpThat file carried a note saying
localize()belonged to an integration suite,because every branch of it goes through
Lang::getTxt(). That is no longer true, sothe note goes and the five tests it stood in for arrive, 7 assertions between them:
each unit pluralised on its own value into a sentence, the unit order not depending
on how the caller wrote the argument, and asking for the total number of days falling
back to years, months and days when the interval has no total rather than producing a
flat "0 days" - the half of #9499 that
toParsable()could only imply with thestrings hard coded. Plus the two edges around an empty result.
The thirteen tests already in the file are untouched.
Why the unit suite rather than the integration one
#9347 extends the test system to cover what genuinely needs a database, and that is
where anything reaching
Db::$dbbelongs. ButLangis mostly string handling andfile loading, and a test that needs no database is far cheaper: this suite finishes
in under a second, runs on every pull request on 8.4 and 8.5, and needs no Docker and
no installed forum behind it. So these belong here, and #9581 is what lets them be
here. The two suites are complements.
Verified
LangTest, and 5 tests / 7 assertions added toTimeIntervalTest. That takes thesuite from 139 tests and 205 assertions to 151 tests and 227 assertions.
--order-by=random, which is the check that matters here,since PHPUnit does not reset SMF's statics between tests and each of these leaves a
loaded language behind.
tearDown()putsLangback to nothing loaded.release-3.0with the database errorabove, and pass on top of [3.0] Removes SMF\Lang's dependency on the database #9581. The 139 that were already there pass either way.
php-cs-fixerclean on both files;check-signed-off.php,check-smf-index.php,check-smf-license.phpandcheck-eof.phpall exit 0.Merge order
Merge #9581 before this one. This branch contains it, so the diff shown here is
its commit as well as the two test commits; once it lands and this is rebased on
release-3.0, what is left istests/Unit/LangTest.phpand thelocalize()tests.Issues References (Fixes|Related|Closes)
localize()behaviour it changed is what the newTimeIntervalTestcases pin.database.