Repository navigation
test(ui): each on-display UI test gets its own config directory, so a --gtest_repeat run never opens on the layout the last one saved (#2932) - #2941
Merged
Conversation
… --gtest_repeat run never opens on the layout the last one saved (#2932) UiScenarioTest and UiLayersOnDisplayTest now derive from TestSupport::ScratchConfigDirTest (tests/App/ScratchConfigDir.h). SetUp() makes a new, empty temporary directory and points both Core::ConfigDirOverride::active() (new setActiveForTesting() seam; what each Core::Application's PathService reads) and UserConfig (resetConfigPathForTesting(): config.toml, settings back to defaults) at it. TearDown() restores both and the settings, then removes the directory; resetConfigPathForTesting() first waits for a queued or in-flight background save (#2916), so nothing writes into the directory once it is gone. With isolation, #2931's narrowing workaround in the column-resized step is gone: PID always starts at its default width and is widened 40 px. Before (workaround disabled, shared config): --gtest_repeat=30 passed runs 1-18, then failed 19, 21, 23, 25, 27 and 29 (PID 826 px and wider, its right border out of view). After: 30/30. Fixes #2932 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
Closed
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
Each
--gtest_repeatiteration ofUiScenarioTestused to open on the Processes table layout the previous iteration saved to config.toml on close. PID got 40 px wider every time until its right border was out of view, and from then on every other iteration failed. #2931 worked around this in the resize step by narrowing PID once it was wide. This PR isolates each test's config instead.tests/App/ScratchConfigDir.h(new):TestSupport::ScratchConfigDirTest, a gtest fixture.Core::ConfigDirOverride::active()at that directory (eachCore::Application'sPathServicereads it; the user themes folder lives there). It also pointsUserConfigat it withresetConfigPathForTesting(), so config.toml is read from there and every setting starts at its default.TASKSMACK_CONFIG_DIRis read once per process (active()caches it, andUserConfigis a singleton that keeps the path it built from it), so setting the variable again would change nothing. The variable itself isn't touched, so it has nothing to restore.resetConfigPathForTesting()waits for a background save that is queued or being written before it repoints, so nothing can write into the directory after TearDown removes it.ShellLayer::onDetach()has also already saved synchronously by then, because the test'sCore::Applicationis destroyed inside the test body, before TearDown runs.src/Core/ConfigDirOverride.{h,cpp}: the valueactive()returns is now a function-local static that can be changed, behind a newsetActiveForTesting()seam. It is documented as test-only and not thread-safe (call it only while no Application or worker is running). Behaviour without the seam is the same: the variable is read on first use, the path is made absolute and the directory is created.UiScenarioTestandUiLayersOnDisplayTest: both now derive from the fixture, soTESTbecomesTEST_F. Suite names don't change, so the ctestSUITE_PROPERTIES(synthetic machine, RUN_SERIAL) still apply.UiLayersOnDisplayTestruns in the same process and shares the sameUserConfig, and three of its tests save or read config there. Its first-run golden test no longer has to empty the shared directory by hand: it now checks that the fixture gave it an empty one.column-resizedstep no longer narrows PID. With isolation, PID always starts at its default width, so the step always widens it by 40 px. Nothing else needed the workaround.Fixes #2932
PR Size
Medium: 210 lines changed (170 added, 40 removed), most of it the new fixture header and the 15
TESTtoTEST_Frenames.Type of Change
src/change is a test-only seam)Checklist
clang-tidyand addressed any warnings (on the touched.cppfiles: no findings in the new or changed code. The local clang-tidy also reportsreadability-trailing-commaon unchangedsteps.push_back({...})initializers and MSVC STL analyzer noise, both already there on main.)clang-formaton my changespre-commit run --all-filesor installed pre-commit hooks (pre-commit run --fileson the touched files, plus the commit hook)Testing
Windows 11,
win-debug:--gtest_repeat=30column-resized: PID is 826 px wide on run 19 and 40 px wider on each failure after, its right border at x=1304 against the table's 1234.--gtest_repeat=30tasksmack_test_config_*directories left in %TEMP%ctest -R UiScenarioTestUiLayersOnDisplayTestonce (ctest) and--gtest_repeat=3AccessNoticeOnDisplayTesttests skipped, as on main (they need an Administrator session)ConfigDirOverrideTest,EnvHooksTestAdditional Notes
ScreenshotOnDisplayTest,AccessNoticeOnDisplayTest,SettingsThemeApplyTest) still use AppTestsMain.cpp's directory, as before. The fixture can be adopted there later if they ever need it.🤖 Generated with Claude Code