Skip to content

Commit 25d6c78

Browse files
committed
refactor(test): defer generator lookup until the per-test settings load
Generator lookup happened once at `TestRunner` construction. With data-driven Handlebars generators now possible, a generator contributed via a test's `addons-supplemental` was unreachable from the test binary: the lookup ran before that test's mrdocs.yml had been loaded, so the generator wasn't yet registered. The runner now defers the lookup. The built-in generators are not affected.
1 parent 47db0f5 commit 25d6c78

7 files changed

Lines changed: 146 additions & 26 deletions

File tree

src/lib/Gen/hbs/AddonPaths.hpp

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
55
//
66
// Copyright (c) 2025 Alan de Freitas (alandefreitas@gmail.com)
7+
// Copyright (c) 2026 Gennaro Prota (gennaro.prota@gmail.com)
78
//
89
// Official repository: https://github.com/cppalliance/mrdocs
910
//
@@ -21,6 +22,36 @@
2122

2223
namespace mrdocs::hbs::addon_paths {
2324

25+
/** Returns the list of addon root directories from the configuration.
26+
27+
This function collects all valid addon root paths by checking
28+
the primary addons directory and any supplemental addon directories
29+
specified in the settings.
30+
31+
@param settings The configuration settings containing addon paths.
32+
@return A vector of existing addon root directory paths. The primary
33+
addons directory (if it exists) appears first, followed by
34+
any existing supplemental addon directories in their
35+
configured order.
36+
*/
37+
inline std::vector<std::string>
38+
addonRoots(Config::Settings const& settings)
39+
{
40+
std::vector<std::string> roots;
41+
roots.reserve(1 + settings.addonsSupplemental.size());
42+
43+
if (files::exists(settings.addons))
44+
roots.push_back(settings.addons);
45+
46+
for (auto const& supplemental : settings.addonsSupplemental)
47+
{
48+
if (files::exists(supplemental))
49+
roots.push_back(supplemental);
50+
}
51+
return roots;
52+
}
53+
54+
2455
/** Returns directories containing Handlebars partial templates.
2556
2657
For each addon root, this function looks for partial templates in:
@@ -133,7 +164,7 @@ findFile(
133164
std::string_view subdir,
134165
std::string_view filename)
135166
{
136-
auto roots = mrdocs::addonRoots(config);
167+
auto roots = addonRoots(config.settings());
137168
for (auto it = roots.rbegin(); it != roots.rend(); ++it)
138169
{
139170
std::string candidate = files::appendPath(*it, "generator", generator, subdir, filename);

src/lib/Gen/hbs/Builder.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ Builder(
454454
namespace fs = std::filesystem;
455455

456456
auto const& config = domCorpus->config;
457-
auto const roots = addonRoots(config);
457+
auto const roots = addon_paths::addonRoots(config.settings());
458458
auto const partialDirs = addon_paths::partialDirs(roots, domCorpus.fileExtension);
459459
auto const helperDirs = addon_paths::helperDirs(roots, domCorpus.fileExtension);
460460
auto const layoutDirs = addon_paths::layoutDirs(roots, domCorpus.fileExtension);

src/test/Support/TestLayout.cpp

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -28,26 +28,46 @@ pathWithExtension(
2828

2929
} // (anon)
3030

31-
/** Build the per-file layout and normalized settings with mode validation. */
32-
Expected<ResolvedLayout>
33-
resolveTestLayout(
31+
/** Read any per-file mrdocs.yml on top of the directory-level settings. */
32+
Expected<LoadedTestSettings>
33+
loadTestSettings(
3434
llvm::StringRef filePath,
3535
Config::Settings const& dirSettings,
36-
llvm::StringRef generatorExtension,
37-
ReferenceDirectories const& dirs,
38-
Action action)
36+
ReferenceDirectories const& dirs)
3937
{
40-
Config::Settings fileSettings = dirSettings;
41-
auto configPath = files::withExtension(filePath, "yml");
42-
bool const hasFileConfig = files::exists(configPath);
43-
if (hasFileConfig)
38+
LoadedTestSettings result;
39+
result.settings = dirSettings;
40+
result.dirMultipage = dirSettings.multipage;
41+
std::string const configPath = files::withExtension(filePath, "yml");
42+
result.hasFileConfig = files::exists(configPath);
43+
if (result.hasFileConfig)
4444
{
45-
if (auto exp = Config::Settings::load_file(fileSettings, configPath, dirs); !exp)
45+
Expected<void> const exp = Config::Settings::load_file(
46+
result.settings, configPath, dirs);
47+
if (!exp)
4648
{
4749
return Unexpected(exp.error());
4850
}
4951
}
52+
return result;
53+
}
5054

55+
/** Build the layout, prepare multipage outputs, and normalize settings.
56+
The split from loadTestSettings lets the caller run addon-generator
57+
discovery in between, so the chosen generator's file extension is
58+
known when paths are computed.
59+
*/
60+
Expected<ResolvedLayout>
61+
buildTestLayout(
62+
llvm::StringRef filePath,
63+
LoadedTestSettings loaded,
64+
llvm::StringRef generatorExtension,
65+
ReferenceDirectories const& dirs,
66+
Action action)
67+
{
68+
bool const dirMultipage = loaded.dirMultipage;
69+
Config::Settings fileSettings = std::move(loaded.settings);
70+
bool const hasFileConfig = loaded.hasFileConfig;
5171
bool const hasTagfileOverride = !fileSettings.tagfile.empty();
5272

5373
TestLayout layout;
@@ -118,7 +138,7 @@ resolveTestLayout(
118138
return Unexpected(Error("multipage tests require a per-file mrdocs.yml with multipage: true"));
119139
}
120140

121-
if (dirSettings.multipage)
141+
if (dirMultipage)
122142
{
123143
return Unexpected(Error("multipage defaults must remain disabled at the directory level"));
124144
}

src/test/Support/TestLayout.hpp

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,42 @@ struct ResolvedLayout
4343
TestLayout layout;
4444
};
4545

46-
/** Resolve per-test settings + layout, enforcing single vs multipage rules. */
47-
Expected<ResolvedLayout>
48-
resolveTestLayout(
46+
/** Settings produced by loadTestSettings before the layout is built.
47+
*/
48+
struct LoadedTestSettings
49+
{
50+
Config::Settings settings;
51+
/// True if a per-file mrdocs.yml was found and merged.
52+
bool hasFileConfig = false;
53+
/// Snapshot of the directory-level multipage flag before merging.
54+
/// Used to enforce that multipage may only be enabled at the
55+
/// per-file level.
56+
bool dirMultipage = false;
57+
};
58+
59+
/** Load any per-file mrdocs.yml on top of the directory-level settings.
60+
61+
No layout work is done here: the per-file settings are needed before
62+
the test's generator is known, so addon discovery can run against
63+
the merged addons paths.
64+
*/
65+
Expected<LoadedTestSettings>
66+
loadTestSettings(
4967
llvm::StringRef filePath,
5068
Config::Settings const& dirSettings,
69+
ReferenceDirectories const& dirs);
70+
71+
/** Build the per-file layout from already-loaded settings.
72+
73+
Computes expected-output paths, applies multipage handling (creating
74+
the temporary output directory and adjusting the settings' output and
75+
tagfile fields), normalizes the settings, and validates the
76+
single vs multipage invariants.
77+
*/
78+
Expected<ResolvedLayout>
79+
buildTestLayout(
80+
llvm::StringRef filePath,
81+
LoadedTestSettings loaded,
5182
llvm::StringRef generatorExtension,
5283
ReferenceDirectories const& dirs,
5384
Action action);

src/test/TestRunner.cpp

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
//
66
// Copyright (c) 2023 Vinnie Falco (vinnie.falco@gmail.com)
77
// Copyright (c) 2023 Alan de Freitas (alandefreitas@gmail.com)
8+
// Copyright (c) 2026 Gennaro Prota (gennaro.prota@gmail.com)
89
//
910
// Official repository: https://github.com/cppalliance/mrdocs
1011
//
@@ -17,6 +18,7 @@
1718
#include "Support/Comparison.hpp"
1819
#include <lib/ConfigImpl.hpp>
1920
#include <lib/CorpusImpl.hpp>
21+
#include <lib/Gen/hbs/DataDrivenGenerators.hpp>
2022
#include <lib/MrDocsCompilationDatabase.hpp>
2123
#include <lib/SingleFileDB.hpp>
2224
#include <lib/Support/ExecuteAndWaitWithLogging.hpp>
@@ -37,9 +39,10 @@ namespace mrdocs {
3739

3840
TestRunner::
3941
TestRunner(std::string_view generator)
40-
: gen_(findGenerator(generator))
42+
: genId_(generator)
4143
{
42-
MRDOCS_ASSERT(gen_ != nullptr);
44+
// The generator is looked up per-test in `handleFile`; after that,
45+
// test's mrdocs.yml has been loaded and addon discovery has run.
4346
}
4447

4548
namespace {
@@ -165,8 +168,37 @@ handleFile(
165168
if (!ensureRegularCpp(filePath))
166169
return;
167170

168-
auto resolved = resolveTestLayout(
169-
filePath, dirSettings, gen_->fileExtension(), dirs_, testArgs.action);
171+
// Load the per-file mrdocs.yml first so data-driven generators
172+
// contributed via addons-supplemental are visible to discovery
173+
// before the chosen generator is looked up.
174+
//
175+
// The generator registry is process-global and persists across
176+
// tests. `discoverDataDrivenGenerators` is idempotent (it skips ids
177+
// already installed), so re-running it per fixture is safe; but
178+
// it also means the first fixture that registers a given id
179+
// wins, and a later fixture that ships a generator directory
180+
// with the same id will see its own contents quietly ignored.
181+
Expected<LoadedTestSettings> loaded =
182+
loadTestSettings(filePath, dirSettings, dirs_);
183+
if (!loaded)
184+
{
185+
return report::error("{}: \"{}\"", loaded.error(), filePath);
186+
}
187+
Expected<void> discovered =
188+
hbs::discoverDataDrivenGenerators(loaded->settings);
189+
if (!discovered)
190+
{
191+
return report::error("{}: \"{}\"", discovered.error(), filePath);
192+
}
193+
Generator const* gen = findGenerator(genId_);
194+
if (!gen)
195+
{
196+
return report::error(
197+
"{}: the Generator \"{}\" was not found", filePath, genId_);
198+
}
199+
200+
Expected<ResolvedLayout> resolved = buildTestLayout(
201+
filePath, *std::move(loaded), gen->fileExtension(), dirs_, testArgs.action);
170202
if (!resolved)
171203
{
172204
return report::error("{}: \"{}\"", resolved.error(), filePath);
@@ -194,7 +226,7 @@ handleFile(
194226
db,
195227
config,
196228
defaultIncludePaths);
197-
handleCompilationDatabase(filePath, compilations, config, layout);
229+
handleCompilationDatabase(filePath, *gen, compilations, config, layout);
198230
};
199231

200232
runWith({ "clang", "-std=c++23" });
@@ -204,6 +236,7 @@ handleFile(
204236
void
205237
TestRunner::handleCompilationDatabase(
206238
llvm::StringRef filePath,
239+
Generator const& gen,
207240
MrDocsCompilationDatabase const& compilations,
208241
std::shared_ptr<ConfigImpl const> const& config,
209242
TestLayout const& layout)
@@ -219,7 +252,7 @@ TestRunner::handleCompilationDatabase(
219252
{
220253
test_support::SinglePageArgs args{
221254
layout,
222-
*gen_,
255+
gen,
223256
**corpus,
224257
filePath,
225258
testArgs.action,
@@ -237,7 +270,7 @@ TestRunner::handleCompilationDatabase(
237270
{
238271
test_support::MultipageArgs args{
239272
layout,
240-
*gen_,
273+
gen,
241274
**corpus,
242275
testArgs.action,
243276
testArgs.forceOption.getValue(),

src/test/TestRunner.hpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
//
66
// Copyright (c) 2023 Vinnie Falco (vinnie.falco@gmail.com)
77
// Copyright (c) 2023 Alan de Freitas (alandefreitas@gmail.com)
8+
// Copyright (c) 2026 Gennaro Prota (gennaro.prota@gmail.com)
89
//
910
// Official repository: https://github.com/cppalliance/mrdocs
1011
//
@@ -55,7 +56,10 @@ struct TestResults
5556
class TestRunner
5657
{
5758
ThreadPool threadPool_;
58-
Generator const* gen_;
59+
/// Id of the chosen generator. Resolved per-test (after each test's
60+
/// settings load) so that data-driven generators contributed via
61+
/// addons-supplemental are picked up correctly.
62+
std::string genId_;
5963
ReferenceDirectories dirs_;
6064

6165
/** Run a single .cpp test file with inherited directory settings. */
@@ -80,6 +84,7 @@ class TestRunner
8084
void
8185
handleCompilationDatabase(
8286
llvm::StringRef filePath,
87+
Generator const& gen,
8388
MrDocsCompilationDatabase const& compilations,
8489
std::shared_ptr<ConfigImpl const> const& config,
8590
TestLayout const& layout);

src/tool/GenerateAction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ DoGenerateAction(
5656
// Handlebars layouts is registered as an additional generator
5757
// (subject to id and layout-template checks) before the user-
5858
// requested generator is looked up below.
59-
MRDOCS_TRY(hbs::discoverDataDrivenGenerators(*config));
59+
MRDOCS_TRY(hbs::discoverDataDrivenGenerators(config->settings()));
6060

6161
// --------------------------------------------------------------
6262
//

0 commit comments

Comments
 (0)