From c7a2f4c393a1043c9250eddfe85a01e37ef528e0 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Sat, 12 Sep 2026 14:18:56 -0400 Subject: [PATCH 01/10] Make CeedlingEngine unit-testable; characterize current YAML/mixin behavior CeedlingEngine had zero unit coverage. Its process spawning, YAML reads, and workspace-config/diagnostics access all went straight to child_process, fs, and a live vscode module - unreachable from a plain Node test process. Inject exec, readFile, workspace-config resolution, and the ProblemMatcher instance instead of touching those directly. Logger, ProblemMatcher, and vscode itself become type-only imports here - each has real runtime vscode calls of its own, and importing only their types keeps this file's module graph free of any require('vscode'). testController.ts, which already runs inside the Extension Host, constructs the real instances and supplies them. Add a baseline unit suite pinning down current getYmlProjectData, mergeYmlProjectData, checkYmlProjectData, and execCeedling flag-construction behavior - including two known quirks (an empty project.yml resolving undefined instead of {}, and a stale ceedlingExplorer.projectPath setting name in an error message) kept as documented baseline, not fixed here. Step 1 of the dumpconfig/mixins plan: establish coverage for current behavior before replacing it. Co-Authored-By: Claude Sonnet 5 --- src/ceedlingEngine.ts | 45 ++++-- src/testController.ts | 6 +- tests/unit/ceedlingEngine.test.ts | 238 ++++++++++++++++++++++++++++++ 3 files changed, 279 insertions(+), 10 deletions(-) create mode 100644 tests/unit/ceedlingEngine.test.ts diff --git a/src/ceedlingEngine.ts b/src/ceedlingEngine.ts index f08431e..2694633 100644 --- a/src/ceedlingEngine.ts +++ b/src/ceedlingEngine.ts @@ -8,7 +8,11 @@ import semver from 'semver'; import stripAnsi from 'strip-ansi'; import tree_kill from 'tree-kill'; import util from 'util'; -import vscode from 'vscode'; +// Type-only. This file has no runtime dependency on the `vscode` module - the one real call it +// used to make, resolving workspace configuration, is now supplied by the caller (see +// `configProvider` below). That keeps this file loadable in a plain Node test process, where the +// `vscode` module the Extension Host normally injects does not exist. +import type vscode from 'vscode'; import xml2js from 'xml2js'; import { buildFileLabelRegex, @@ -22,8 +26,12 @@ import { testCaseFilterMatchedExactly, } from './ceedlingOutputParsing'; import { getTestListFromXmlReport } from './ceedlingXmlReport'; -import { Logger } from './logger'; -import { FileDiagnostic, ProblemMatcher, ProblemMatchingPattern } from './problemMatcher'; +// Type-only, same reasoning as the `vscode` import above: Logger and ProblemMatcher both have +// real runtime vscode calls of their own (an output channel, a DiagnosticCollection). Importing +// only their types keeps this file from pulling either module's code in at require-time. Callers +// construct both and hand this file the instances (see `configProvider`/`deps.problemMatcher`). +import type { Logger } from './logger'; +import type { FileDiagnostic, ProblemMatcher, ProblemMatchingPattern } from './problemMatcher'; const MINIMUM_CEEDLING_VERSION = '1.0.0'; const TEST_VERBOSITY_FLAG = '--verbosity debug'; @@ -123,12 +131,31 @@ export class CeedlingEngine { readonly problemMatcher: ProblemMatcher; + // Process spawning and YAML file reads go through these fields instead of the + // child_process/fs modules directly. Tests substitute fakes here to exercise + // Ceedling-invocation and config-parsing logic without a real Ceedling install or disk I/O. + private readonly exec: typeof child_process.exec; + private readonly readFile: typeof fs.readFile; + // Resolves workspace configuration. Required, no in-file default - the only real + // implementation needs the `vscode` module, and this file otherwise has none. + private readonly configProvider: (uri: vscode.Uri) => vscode.WorkspaceConfiguration; + constructor( public readonly workspaceFolder: vscode.WorkspaceFolder, private readonly logger: Logger, private readonly isDevelopmentMode: boolean, + deps: { + configProvider: (uri: vscode.Uri) => vscode.WorkspaceConfiguration, + // Constructed by the caller, not here - see the type-only problemMatcher import above. + problemMatcher: ProblemMatcher, + exec?: typeof child_process.exec, + readFile?: typeof fs.readFile, + }, ) { - this.problemMatcher = new ProblemMatcher(logger); + this.problemMatcher = deps.problemMatcher; + this.exec = deps.exec ?? child_process.exec; + this.readFile = deps.readFile ?? fs.readFile; + this.configProvider = deps.configProvider; } dispose(): void { @@ -137,7 +164,7 @@ export class CeedlingEngine { } getConfiguration(): vscode.WorkspaceConfiguration { - return vscode.workspace.getConfiguration('ceedlingExplorer', this.workspaceFolder.uri); + return this.configProvider(this.workspaceFolder.uri); } getDebugLaunchConfig(projectKey: string): string { @@ -584,7 +611,7 @@ export class CeedlingEngine { const shell = this.getShellPath(); this.logger.debug(`execCeedling(args=${util.format(args)}) \ncommand=${command} \ncwd=${cwd} \nshell=${shell}`); return new Promise((resolve) => { - this.ceedlingProcess = child_process.exec( + this.ceedlingProcess = this.exec( command, { cwd: cwd, @@ -804,7 +831,7 @@ export class CeedlingEngine { } return new Promise((resolve) => { const project_yml = this.getYmlProjectPath(projectKey); - fs.readFile(project_yml, 'utf8', (error, data) => { + this.readFile(project_yml, 'utf8', (error, data) => { if (error) { this.logger.error(`Failed to read YAML file '${project_yml}': ${util.format(error)}`); resolve(undefined); @@ -828,7 +855,7 @@ export class CeedlingEngine { private mergeYmlProjectData(projectKey: string): Promise { return new Promise((resolve) => { const project_data = this.getYmlProjectPath(projectKey); - fs.readFile(project_data, 'utf8', (error, data) => { + this.readFile(project_data, 'utf8', (error, data) => { if (error) { this.logger.error(`Failed to read YAML file: ${util.format(error)}`); resolve(undefined); @@ -837,7 +864,7 @@ export class CeedlingEngine { try { const result = yaml.safeLoad(data); const defaultYmlPath = path.resolve(this.projectData[projectKey].absPath, 'project.yml'); - fs.readFile(defaultYmlPath, 'utf8', (error, data) => { + this.readFile(defaultYmlPath, 'utf8', (error, data) => { if (error) { this.logger.error(`Failed to read default YAML file: ${util.format(error)}`); resolve(result); diff --git a/src/testController.ts b/src/testController.ts index 9fca13c..f4d4b8a 100644 --- a/src/testController.ts +++ b/src/testController.ts @@ -12,6 +12,7 @@ import { } from './ceedlingEngine'; import { extractGdbLogReference } from './ceedlingOutputParsing'; import { Logger } from './logger'; +import { ProblemMatcher } from './problemMatcher'; type ItemMeta = | { kind: 'project', projectKey: string } @@ -44,7 +45,10 @@ export class CeedlingTestController { private readonly isDevelopmentMode: boolean, ) { this.logger.debug(`CeedlingTestController created for ${workspaceFolder.uri.toString()}`); - this.engine = new CeedlingEngine(workspaceFolder, logger, isDevelopmentMode); + this.engine = new CeedlingEngine(workspaceFolder, logger, isDevelopmentMode, { + configProvider: (uri) => vscode.workspace.getConfiguration('ceedlingExplorer', uri), + problemMatcher: new ProblemMatcher(logger), + }); this.controller = vscode.tests.createTestController( `ceedlingExplorer:${workspaceFolder.uri.toString()}`, 'Ceedling', diff --git a/tests/unit/ceedlingEngine.test.ts b/tests/unit/ceedlingEngine.test.ts new file mode 100644 index 0000000..fa631f1 --- /dev/null +++ b/tests/unit/ceedlingEngine.test.ts @@ -0,0 +1,238 @@ +import * as assert from 'assert'; +import * as child_process from 'child_process'; +import * as fs from 'fs'; +import type vscode from 'vscode'; +import { CeedlingEngine } from '../../src/ceedlingEngine'; +import type { Logger } from '../../src/logger'; +import type { ProblemMatcher } from '../../src/problemMatcher'; + +// Characterizes CeedlingEngine's current YAML-loading and command-construction behavior ahead of +// replacing it with `ceedling dumpconfig` (see the project plan). None of this is new behavior - +// every assertion here, including the ones that look like bugs, documents what the code already +// does, so the refactor has a baseline to diff against. + +function fakeWorkspaceFolder(fsPath: string): vscode.WorkspaceFolder { + return { + uri: { fsPath, toString: () => `file://${fsPath}` }, + name: 'fake', + index: 0, + } as unknown as vscode.WorkspaceFolder; +} + +function noopLogger(): Logger { + return { + trace() { }, debug() { }, info() { }, warn() { }, error() { }, + showInfo() { }, showError() { }, dispose() { }, + } as unknown as Logger; +} + +function fakeConfig(settings: Record): vscode.WorkspaceConfiguration { + return { + get: (key: string, def?: any) => (key in settings ? settings[key] : def), + } as unknown as vscode.WorkspaceConfiguration; +} + +function fakeProblemMatcher(): ProblemMatcher { + return { + scan: () => [], + dispose() { }, + clear() { }, + setActualIds() { }, + } as unknown as ProblemMatcher; +} + +// Keyed by absolute path. A missing key simulates an ENOENT read failure. +function fakeReadFile(files: Record): typeof fs.readFile { + return ((filePath: string, _encoding: any, callback: any) => { + if (Object.prototype.hasOwnProperty.call(files, filePath)) { + callback(null, files[filePath]); + } else { + callback(Object.assign(new Error(`ENOENT: no such file, open '${filePath}'`), { code: 'ENOENT' }), undefined); + } + }) as unknown as typeof fs.readFile; +} + +// Records every invocation instead of actually spawning anything; resolves each call +// with an empty successful result unless a call-specific result is queued. +function fakeExec(calls: { command: string, options: any }[]): typeof child_process.exec { + return ((command: string, options: any, callback: any) => { + calls.push({ command, options }); + callback(null, '', ''); + return {} as child_process.ChildProcess; + }) as unknown as typeof child_process.exec; +} + +function makeEngine(opts: { + exec?: typeof child_process.exec, + readFile?: typeof fs.readFile, + settings?: Record, + isDevelopmentMode?: boolean, +} = {}): CeedlingEngine { + return new CeedlingEngine( + fakeWorkspaceFolder('/ws'), + noopLogger(), + opts.isDevelopmentMode ?? false, + { + configProvider: () => fakeConfig(opts.settings ?? {}), + problemMatcher: fakeProblemMatcher(), + exec: opts.exec, + readFile: opts.readFile, + }, + ); +} + +// Installs one project directly on the engine's private project map, bypassing +// loadProjectPaths() (which needs real fs.existsSync/lstatSync this seam doesn't cover). +function withProject(engine: CeedlingEngine, key: string, data: { absPath: string, ymlFileName: string }) { + (engine as any).projectData = { + [key]: { debugLaunchConfig: 'ceedling', projectPath: '.', files: {}, ...data }, + }; +} + +suite('getYmlProjectData (project.yml only, no mixin)', () => { + test('parses the file and resolves the plain object', async () => { + const engine = makeEngine({ + readFile: fakeReadFile({ '/ws/proj/project.yml': ':project:\n :build_root: build\n' }), + }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + const result = await (engine as any).getYmlProjectData('proj'); + assert.deepStrictEqual(result, { ':project': { ':build_root': 'build' } }); + }); + + test('resolves undefined when the file cannot be read', async () => { + const engine = makeEngine({ readFile: fakeReadFile({}) }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + const result = await (engine as any).getYmlProjectData('proj'); + assert.strictEqual(result, undefined); + }); + + test('resolves undefined when the file fails to parse', async () => { + const engine = makeEngine({ + readFile: fakeReadFile({ '/ws/proj/project.yml': ':project: [\n broken' }), + }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + const result = await (engine as any).getYmlProjectData('proj'); + assert.strictEqual(result, undefined); + }); + + // Current quirk, not a design choice: an empty project.yml parses to `undefined` (js-yaml's + // safeLoad on empty input), and nothing here guards it back to `{}` on this code path. + test('quirk: an empty project.yml resolves undefined, not {}', async () => { + const engine = makeEngine({ readFile: fakeReadFile({ '/ws/proj/project.yml': '' }) }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + const result = await (engine as any).getYmlProjectData('proj'); + assert.strictEqual(result, undefined); + }); +}); + +suite('getYmlProjectData -> mergeYmlProjectData (project points at an alternate/mixin yml)', () => { + test('deep-merges the mixin over the base project.yml, mixin winning on scalar conflicts', async () => { + const engine = makeEngine({ + readFile: fakeReadFile({ + '/ws/proj/mixin.yml': ':project:\n :build_root: mixin_build\n:plugins:\n :enabled:\n - mixin_plugin\n', + '/ws/proj/project.yml': ':project:\n :build_root: base_build\n :test_file_prefix: test_\n:plugins:\n :enabled:\n - base_plugin\n', + }), + }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'mixin.yml' }); + const result = await (engine as any).getYmlProjectData('proj'); + // Scalar: mixin's value replaces the base's. + assert.strictEqual(result[':project'][':build_root'], 'mixin_build'); + // Untouched base key survives the merge. + assert.strictEqual(result[':project'][':test_file_prefix'], 'test_'); + // Lists concatenate base-then-mixin; deepmerge does not de-duplicate or reorder. + assert.deepStrictEqual(result[':plugins'][':enabled'], ['base_plugin', 'mixin_plugin']); + }); + + test('resolves the mixin alone when the base project.yml cannot be read', async () => { + const engine = makeEngine({ + readFile: fakeReadFile({ '/ws/proj/mixin.yml': ':project:\n :build_root: mixin_build\n' }), + }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'mixin.yml' }); + const result = await (engine as any).getYmlProjectData('proj'); + assert.deepStrictEqual(result, { ':project': { ':build_root': 'mixin_build' } }); + }); + + test('resolves undefined when the mixin itself cannot be read (base is never even attempted)', async () => { + const engine = makeEngine({ + readFile: fakeReadFile({ '/ws/proj/project.yml': ':project:\n :build_root: base_build\n' }), + }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'mixin.yml' }); + const result = await (engine as any).getYmlProjectData('proj'); + assert.strictEqual(result, undefined); + }); +}); + +suite('checkYmlProjectData', () => { + test('reports a load failure, referencing the settings name as it stands today (a known-wrong name)', async () => { + const engine = makeEngine({ readFile: fakeReadFile({}) }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + const error = await (engine as any).checkYmlProjectData('proj'); + // Characterizes the pre-existing bug: the real setting is `ceedlingExplorer.projects`, + // not `ceedlingExplorer.projectPath`. The upcoming rewrite corrects this message. + assert.match(error, /ceedlingExplorer\.projectPath/); + }); + + test('reports the plugin-not-enabled error when report_tests_log_factory is missing', async () => { + const engine = makeEngine({ + readFile: fakeReadFile({ '/ws/proj/project.yml': ':plugins:\n :enabled:\n - other_plugin\n' }), + }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + const error = await (engine as any).checkYmlProjectData('proj'); + assert.match(error, /report_tests_log_factory.*is not enabled/); + }); + + test('reports nothing when report_tests_log_factory is enabled', async () => { + const engine = makeEngine({ + readFile: fakeReadFile({ '/ws/proj/project.yml': ':plugins:\n :enabled:\n - report_tests_log_factory\n' }), + }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + const error = await (engine as any).checkYmlProjectData('proj'); + assert.strictEqual(error, undefined); + }); +}); + +suite('execCeedling: --project/--mixin flag construction', () => { + test('before the version check completes, no --project/--mixin flags are added and cwd is "."', async () => { + const calls: { command: string, options: any }[] = []; + const engine = makeEngine({ exec: fakeExec(calls) }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + await (engine as any).execCeedling(['version']); + assert.strictEqual(calls[0].command, 'ceedling version'); + assert.strictEqual(calls[0].options.cwd, '.'); + }); + + test('once version-checked, a default project.yml gets --project (and cwd set) - note the doubled space, a pre-existing quirk of building the flags as one pre-spaced string', async () => { + const calls: { command: string, options: any }[] = []; + const engine = makeEngine({ exec: fakeExec(calls) }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + (engine as any).ceedlingVersionChecked = true; + await (engine as any).execCeedling(['test:all'], 'proj'); + assert.strictEqual(calls[0].command, 'ceedling test:all --project project.yml'); + assert.strictEqual(calls[0].options.cwd, '/ws/proj'); + }); + + test('an alternate/mixin yml file name adds a single --mixin flag after --project', async () => { + const calls: { command: string, options: any }[] = []; + const engine = makeEngine({ exec: fakeExec(calls) }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'ci.yml' }); + (engine as any).ceedlingVersionChecked = true; + await (engine as any).execCeedling(['test:all'], 'proj'); + assert.strictEqual(calls[0].command, 'ceedling test:all --project project.yml --mixin ci.yml'); + }); + + test('the verbosity flag is only spliced on in development mode', async () => { + const calls: { command: string, options: any }[] = []; + const engine = makeEngine({ exec: fakeExec(calls), isDevelopmentMode: false }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + (engine as any).ceedlingVersionChecked = true; + await (engine as any).execCeedling(['test:all'], 'proj', '--verbosity debug'); + assert.doesNotMatch(calls[0].command, /--verbosity/); + + const devCalls: { command: string, options: any }[] = []; + const devEngine = makeEngine({ exec: fakeExec(devCalls), isDevelopmentMode: true }); + withProject(devEngine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + (devEngine as any).ceedlingVersionChecked = true; + await (devEngine as any).execCeedling(['test:all'], 'proj', '--verbosity debug'); + assert.match(devCalls[0].command, /--verbosity debug$/); + }); +}); From 08d4e51ab3ba57fb1dbff86c3bc077083e725a98 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Sat, 12 Sep 2026 14:35:29 -0400 Subject: [PATCH 02/10] Add buildProjectArgs(): pure --project/--mixin flag builder A pure module, no vscode/fs/child_process, matching the existing ceedlingOutputParsing.ts/problemMatching.ts pattern. Encodes Ceedling's own mixin-priority rule (later --mixin wins, lists merge by prepending) as flag order: the project's alternate-yml convention, then user-configured mixins, then the extension's own report-plugin mixin always last. Also quotes any path-like value containing whitespace, since the values about to flow through this (a temp dumpconfig path, the extension's own install path) can contain spaces on Windows and execCeedling's command string had no quoting at all before this. Not yet wired into execCeedling - next commit. Co-Authored-By: Claude Sonnet 5 --- src/ceedlingMixinArgs.ts | 26 +++++++++++++ tests/unit/ceedlingMixinArgs.test.ts | 55 ++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+) create mode 100644 src/ceedlingMixinArgs.ts create mode 100644 tests/unit/ceedlingMixinArgs.test.ts diff --git a/src/ceedlingMixinArgs.ts b/src/ceedlingMixinArgs.ts new file mode 100644 index 0000000..9658433 --- /dev/null +++ b/src/ceedlingMixinArgs.ts @@ -0,0 +1,26 @@ +// Wraps a token in double quotes when it contains whitespace, so a path with spaces - routinely +// true of a Windows install path or the OS temp directory - survives being joined into a single +// shell command string by execCeedling. Every other value here is passed through unquoted, same +// as the literal flags around it. +function quoteIfNeeded(token: string): string { + return /\s/.test(token) ? `"${token}"` : token; +} + +// Builds the --project/--mixin flags common to every Ceedling invocation for a project, in +// Ceedling's own priority order: a later --mixin flag outranks an earlier one, and a mixin's +// lists merge by prepending onto the existing list, not replacing it. Lowest to highest +// priority: the project's own alternate-yml convention (ceedlingExplorer.projects[].path +// pointing at a non-default yml file), the user's configured mixins +// (ceedlingExplorer.mixins/projects[].mixins, already resolved into one ordered list by the +// caller), then the extension's own report-plugin mixin, always last so it always wins. +export function buildProjectArgs(ymlFileName: string, userMixins: readonly string[], bundledMixinPath: string): string[] { + const args = ['--project', 'project.yml']; + if (ymlFileName !== 'project.yml') { + args.push('--mixin', quoteIfNeeded(ymlFileName)); + } + for (const mixin of userMixins) { + args.push('--mixin', quoteIfNeeded(mixin)); + } + args.push('--mixin', quoteIfNeeded(bundledMixinPath)); + return args; +} diff --git a/tests/unit/ceedlingMixinArgs.test.ts b/tests/unit/ceedlingMixinArgs.test.ts new file mode 100644 index 0000000..4b76792 --- /dev/null +++ b/tests/unit/ceedlingMixinArgs.test.ts @@ -0,0 +1,55 @@ +import * as assert from 'assert'; +import { buildProjectArgs } from '../../src/ceedlingMixinArgs'; + +// Ceedling's own mixin rule: a later --mixin flag outranks an earlier one, and lists merge by +// prepending the higher-priority mixin's entries. That makes flag order the whole design here - +// see buildProjectArgs's own comment for the three-tier priority this pins down. + +suite('buildProjectArgs', () => { + test('a default project.yml with no user mixins gets --project and the bundled mixin only', () => { + const args = buildProjectArgs('project.yml', [], '/ext/assets/mixins/report-tests-log-factory.yml'); + assert.deepStrictEqual(args, [ + '--project', 'project.yml', + '--mixin', '/ext/assets/mixins/report-tests-log-factory.yml', + ]); + }); + + test('an alternate/mixin project yml file name is placed lowest priority, right after --project', () => { + const args = buildProjectArgs('ci.yml', [], '/ext/bundled.yml'); + assert.deepStrictEqual(args, [ + '--project', 'project.yml', + '--mixin', 'ci.yml', + '--mixin', '/ext/bundled.yml', + ]); + }); + + test('user-configured mixins are placed between the alternate yml and the bundled mixin, in array order', () => { + const args = buildProjectArgs('ci.yml', ['gcc', 'proj/mixins/coverage.yml'], '/ext/bundled.yml'); + assert.deepStrictEqual(args, [ + '--project', 'project.yml', + '--mixin', 'ci.yml', + '--mixin', 'gcc', + '--mixin', 'proj/mixins/coverage.yml', + '--mixin', '/ext/bundled.yml', + ]); + }); + + test('the bundled mixin is always last, regardless of how many user mixins precede it', () => { + const args = buildProjectArgs('project.yml', ['a', 'b', 'c'], '/ext/bundled.yml'); + assert.strictEqual(args[args.length - 2], '--mixin'); + assert.strictEqual(args[args.length - 1], '/ext/bundled.yml'); + }); + + test('a path-like value containing whitespace is double-quoted; one without is left alone', () => { + const args = buildProjectArgs('project.yml', [], '/Program Files/ext/bundled.yml'); + assert.deepStrictEqual(args, [ + '--project', 'project.yml', + '--mixin', '"/Program Files/ext/bundled.yml"', + ]); + }); + + test('--project itself is never quoted - it is always the literal "project.yml"', () => { + const args = buildProjectArgs('project.yml', [], '/ext/bundled.yml'); + assert.strictEqual(args[1], 'project.yml'); + }); +}); From 6cb5c5faa34bd9ac03e777b025d6ab85db417098 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Sat, 12 Sep 2026 14:39:37 -0400 Subject: [PATCH 03/10] Bundle the report-plugin mixin; add mixin settings; wire execCeedling through it Every Ceedling invocation now gets its --project/--mixin flags from buildProjectArgs() instead of a hand-built, unquoted string. Three sources feed it, lowest to highest priority: a project's own alternate-yml convention, ceedlingExplorer.projects[].mixins (or the workspace-wide ceedlingExplorer.mixins when a project doesn't define its own - absence, not an empty array, is what triggers that fallback), and the extension's own bundled mixin, always last so it always wins. assets/mixins/report-tests-log-factory.yml is that bundled mixin. It force-enables report_tests_log_factory and selects the cppunit report format - enabling the plugin alone doesn't produce the XML report this extension reads. Users no longer need to hand-edit project.yml for this (README update still pending). context.extensionPath threads from activate() through CeedlingTestController into CeedlingEngine to resolve the bundled mixin's absolute path. Existing execCeedling tests updated for the new flag format (no more double-space quirk - buildProjectArgs already returns clean discrete tokens) and extended to cover the new mixins array. Co-Authored-By: Claude Sonnet 5 --- assets/mixins/report-tests-log-factory.yml | 16 +++++++++++ package.json | 16 +++++++++++ src/ceedlingEngine.ts | 29 +++++++++++++++----- src/main.ts | 4 ++- src/testController.ts | 2 ++ tests/unit/ceedlingEngine.test.ts | 31 +++++++++++++++++----- 6 files changed, 84 insertions(+), 14 deletions(-) create mode 100644 assets/mixins/report-tests-log-factory.yml diff --git a/assets/mixins/report-tests-log-factory.yml b/assets/mixins/report-tests-log-factory.yml new file mode 100644 index 0000000..253d382 --- /dev/null +++ b/assets/mixins/report-tests-log-factory.yml @@ -0,0 +1,16 @@ +# Bundled with the extension and always applied as the highest-priority --mixin (see +# buildProjectArgs in src/ceedlingMixinArgs.ts). Guarantees the CppUnit XML report this extension +# parses exists, without requiring a user to hand-edit their own project.yml. +# +# Enabling the plugin alone is not enough - report_tests_log_factory can emit several report +# formats, and :reports: must explicitly select cppunit. See +# https://throwtheswitch.github.io/Ceedling/1.1.0/plugins/report-tests-log-factory/#cppunit-xml-format +# +# :cppunit: :filename: is deliberately left unset. Mixin merge only touches keys a mixin actually +# sets, so a user's own :cppunit: :filename: override (if any) survives untouched. +:plugins: + :enabled: + - report_tests_log_factory +:report_tests_log_factory: + :reports: + - cppunit diff --git a/package.json b/package.json index 5afb693..7c31d20 100644 --- a/package.json +++ b/package.json @@ -110,6 +110,13 @@ "name": { "description": "Name for the project tests folder. If not defined, the yml or folder name is used", "type": "string" + }, + "mixins": { + "markdownDescription": "Ceedling mixins to merge into this project's configuration (mixin names resolved via `:mixins:`/`:load_paths:`, built-in toolchain targets like `gcc`/`clang`, or file paths), lowest to highest priority. When omitted entirely, `ceedlingExplorer.mixins` is used instead for this project; set to `[]` explicitly to opt this project out of that workspace-wide list.", + "type": "array", + "items": { + "type": "string" + } } }, "required": [ @@ -126,6 +133,15 @@ "scope": "resource" } }, + "ceedlingExplorer.mixins": { + "markdownDescription": "Default Ceedling mixins applied to every configured project that does not define its own `mixins` array (see `ceedlingExplorer.projects[].mixins`), lowest to highest priority. The extension's own mixin, which enables the CppUnit XML report this extension depends on, is always applied after these and cannot be overridden.", + "type": "array", + "items": { + "type": "string" + }, + "default": [], + "scope": "resource" + }, "ceedlingExplorer.shellPath": { "description": "Path to the shell where Ceedling is installed", "type": "string", diff --git a/src/ceedlingEngine.ts b/src/ceedlingEngine.ts index 2694633..8998b99 100644 --- a/src/ceedlingEngine.ts +++ b/src/ceedlingEngine.ts @@ -26,6 +26,7 @@ import { testCaseFilterMatchedExactly, } from './ceedlingOutputParsing'; import { getTestListFromXmlReport } from './ceedlingXmlReport'; +import { buildProjectArgs } from './ceedlingMixinArgs'; // Type-only, same reasoning as the `vscode` import above: Logger and ProblemMatcher both have // real runtime vscode calls of their own (an output channel, a DiagnosticCollection). Importing // only their types keeps this file from pulling either module's code in at require-time. Callers @@ -38,9 +39,12 @@ const TEST_VERBOSITY_FLAG = '--verbosity debug'; export type ProjectData = { projectPath: string, - ymlFileName: any, + ymlFileName: string, absPath: string, debugLaunchConfig: string, + // Resolved once in loadProjectPaths(): this project's own ceedlingExplorer.projects[].mixins + // when set, else the workspace-wide ceedlingExplorer.mixins. Lowest to highest priority. + mixins: string[], files: { assembly?: string[], header?: string[], @@ -53,6 +57,9 @@ export type ProjectConfig = { path: string, debugLaunchConfig: string, name?: string, + // Absent (not merely []) means "use the workspace-wide ceedlingExplorer.mixins instead" - + // see loadProjectPaths(). An explicit [] opts this project out of the global list entirely. + mixins?: string[], } export type CeedlingTestCase = { @@ -139,6 +146,9 @@ export class CeedlingEngine { // Resolves workspace configuration. Required, no in-file default - the only real // implementation needs the `vscode` module, and this file otherwise has none. private readonly configProvider: (uri: vscode.Uri) => vscode.WorkspaceConfiguration; + // Absolute path to the extension's bundled mixin that force-enables the CppUnit XML report. + // Always the last, highest-priority --mixin flag on every invocation - see buildProjectArgs. + private readonly bundledMixinPath: string; constructor( public readonly workspaceFolder: vscode.WorkspaceFolder, @@ -148,6 +158,7 @@ export class CeedlingEngine { configProvider: (uri: vscode.Uri) => vscode.WorkspaceConfiguration, // Constructed by the caller, not here - see the type-only problemMatcher import above. problemMatcher: ProblemMatcher, + bundledMixinPath: string, exec?: typeof child_process.exec, readFile?: typeof fs.readFile, }, @@ -156,6 +167,7 @@ export class CeedlingEngine { this.exec = deps.exec ?? child_process.exec; this.readFile = deps.readFile ?? fs.readFile; this.configProvider = deps.configProvider; + this.bundledMixinPath = deps.bundledMixinPath; } dispose(): void { @@ -467,6 +479,10 @@ export class CeedlingEngine { private loadProjectPaths(): void { const projectConfigs = this.getConfiguration().get('projects', []) as Array; + // The workspace-wide fallback. A project's own `mixins` is used only when it's genuinely + // absent from settings (`undefined`) - an explicit `mixins: []` opts a project out of + // this list rather than falling back to it. + const globalMixins = this.getConfiguration().get('mixins', []); this.projectData = {}; let workspacePath = this.workspaceFolder.uri.fsPath; projectConfigs.forEach(projectConfig => { @@ -500,6 +516,7 @@ export class CeedlingEngine { projectPath: projectConfig.path, ymlFileName: ymlName, absPath: absolutePath, + mixins: projectConfig.mixins !== undefined ? projectConfig.mixins : globalMixins, files: {} }; } @@ -510,6 +527,7 @@ export class CeedlingEngine { projectPath: ".", ymlFileName: 'project.yml', absPath: path.resolve(workspacePath, "."), + mixins: globalMixins, files: {} }; } @@ -597,12 +615,9 @@ export class CeedlingEngine { private execCeedling(args: ReadonlyArray, projectKey = Object.keys(this.projectData)[0], verbosity = ''): Promise { let cwd = "."; if (this.ceedlingVersionChecked && projectKey in this.projectData) { - let projectParam = ` --project project.yml`; - if (this.projectData[projectKey].ymlFileName != 'project.yml') { - projectParam += ` --mixin ${this.projectData[projectKey].ymlFileName}`; - } - args = [...args, projectParam]; - cwd = this.projectData[projectKey].absPath; + const project = this.projectData[projectKey]; + args = [...args, ...buildProjectArgs(project.ymlFileName, project.mixins, this.bundledMixinPath)]; + cwd = project.absPath; } if (this.isDevelopmentMode && verbosity) { args = [...args, verbosity]; diff --git a/src/main.ts b/src/main.ts index 8aeb611..8317181 100644 --- a/src/main.ts +++ b/src/main.ts @@ -5,10 +5,11 @@ import { Logger } from './logger'; const logger = new Logger(); const controllers = new Map(); let isDevelopmentMode = false; +let extensionPath = ''; function addWorkspaceFolder(workspaceFolder: vscode.WorkspaceFolder): void { logger.debug(`addWorkspaceFolder(${workspaceFolder.uri.toString()})`); - controllers.set(workspaceFolder, new CeedlingTestController(workspaceFolder, logger, isDevelopmentMode)); + controllers.set(workspaceFolder, new CeedlingTestController(workspaceFolder, logger, isDevelopmentMode, extensionPath)); } function removeWorkspaceFolder(workspaceFolder: vscode.WorkspaceFolder): void { @@ -40,6 +41,7 @@ function debugTestExecutableGuard(): never { export async function activate(context: vscode.ExtensionContext) { isDevelopmentMode = context.extensionMode === vscode.ExtensionMode.Development; + extensionPath = context.extensionPath; const workspaceFolderCount = vscode.workspace.workspaceFolders?.length ?? 0; logger.debug(`activate(): isDevelopmentMode=${isDevelopmentMode}, workspaceFolders=${workspaceFolderCount}`); context.subscriptions.push(logger); diff --git a/src/testController.ts b/src/testController.ts index f4d4b8a..9baaae7 100644 --- a/src/testController.ts +++ b/src/testController.ts @@ -43,11 +43,13 @@ export class CeedlingTestController { public readonly workspaceFolder: vscode.WorkspaceFolder, private readonly logger: Logger, private readonly isDevelopmentMode: boolean, + extensionPath: string, ) { this.logger.debug(`CeedlingTestController created for ${workspaceFolder.uri.toString()}`); this.engine = new CeedlingEngine(workspaceFolder, logger, isDevelopmentMode, { configProvider: (uri) => vscode.workspace.getConfiguration('ceedlingExplorer', uri), problemMatcher: new ProblemMatcher(logger), + bundledMixinPath: path.join(extensionPath, 'assets', 'mixins', 'report-tests-log-factory.yml'), }); this.controller = vscode.tests.createTestController( `ceedlingExplorer:${workspaceFolder.uri.toString()}`, diff --git a/tests/unit/ceedlingEngine.test.ts b/tests/unit/ceedlingEngine.test.ts index fa631f1..c59a6c6 100644 --- a/tests/unit/ceedlingEngine.test.ts +++ b/tests/unit/ceedlingEngine.test.ts @@ -62,11 +62,14 @@ function fakeExec(calls: { command: string, options: any }[]): typeof child_proc }) as unknown as typeof child_process.exec; } +const FAKE_BUNDLED_MIXIN_PATH = '/ext/assets/mixins/report-tests-log-factory.yml'; + function makeEngine(opts: { exec?: typeof child_process.exec, readFile?: typeof fs.readFile, settings?: Record, isDevelopmentMode?: boolean, + bundledMixinPath?: string, } = {}): CeedlingEngine { return new CeedlingEngine( fakeWorkspaceFolder('/ws'), @@ -75,6 +78,7 @@ function makeEngine(opts: { { configProvider: () => fakeConfig(opts.settings ?? {}), problemMatcher: fakeProblemMatcher(), + bundledMixinPath: opts.bundledMixinPath ?? FAKE_BUNDLED_MIXIN_PATH, exec: opts.exec, readFile: opts.readFile, }, @@ -83,9 +87,9 @@ function makeEngine(opts: { // Installs one project directly on the engine's private project map, bypassing // loadProjectPaths() (which needs real fs.existsSync/lstatSync this seam doesn't cover). -function withProject(engine: CeedlingEngine, key: string, data: { absPath: string, ymlFileName: string }) { +function withProject(engine: CeedlingEngine, key: string, data: { absPath: string, ymlFileName: string, mixins?: string[] }) { (engine as any).projectData = { - [key]: { debugLaunchConfig: 'ceedling', projectPath: '.', files: {}, ...data }, + [key]: { debugLaunchConfig: 'ceedling', projectPath: '.', files: {}, mixins: [], ...data }, }; } @@ -201,23 +205,38 @@ suite('execCeedling: --project/--mixin flag construction', () => { assert.strictEqual(calls[0].options.cwd, '.'); }); - test('once version-checked, a default project.yml gets --project (and cwd set) - note the doubled space, a pre-existing quirk of building the flags as one pre-spaced string', async () => { + test('once version-checked, a default project.yml gets --project and the bundled report-plugin mixin, cwd set to the project', async () => { const calls: { command: string, options: any }[] = []; const engine = makeEngine({ exec: fakeExec(calls) }); withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); (engine as any).ceedlingVersionChecked = true; await (engine as any).execCeedling(['test:all'], 'proj'); - assert.strictEqual(calls[0].command, 'ceedling test:all --project project.yml'); + assert.strictEqual(calls[0].command, `ceedling test:all --project project.yml --mixin ${FAKE_BUNDLED_MIXIN_PATH}`); assert.strictEqual(calls[0].options.cwd, '/ws/proj'); }); - test('an alternate/mixin yml file name adds a single --mixin flag after --project', async () => { + test('an alternate/mixin yml file name adds its own --mixin flag, lowest priority (right after --project)', async () => { const calls: { command: string, options: any }[] = []; const engine = makeEngine({ exec: fakeExec(calls) }); withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'ci.yml' }); (engine as any).ceedlingVersionChecked = true; await (engine as any).execCeedling(['test:all'], 'proj'); - assert.strictEqual(calls[0].command, 'ceedling test:all --project project.yml --mixin ci.yml'); + assert.strictEqual( + calls[0].command, + `ceedling test:all --project project.yml --mixin ci.yml --mixin ${FAKE_BUNDLED_MIXIN_PATH}`, + ); + }); + + test("the project's configured mixins land between the alternate yml and the bundled mixin, in order", async () => { + const calls: { command: string, options: any }[] = []; + const engine = makeEngine({ exec: fakeExec(calls) }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml', mixins: ['gcc', 'proj/mixins/coverage.yml'] }); + (engine as any).ceedlingVersionChecked = true; + await (engine as any).execCeedling(['test:all'], 'proj'); + assert.strictEqual( + calls[0].command, + `ceedling test:all --project project.yml --mixin gcc --mixin proj/mixins/coverage.yml --mixin ${FAKE_BUNDLED_MIXIN_PATH}`, + ); }); test('the verbosity flag is only spliced on in development mode', async () => { From b3f9885334fd2dc8265c74735ce42eae647bb3ff Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Sat, 12 Sep 2026 14:44:44 -0400 Subject: [PATCH 04/10] Resolve project config via `ceedling dumpconfig`, not a hand-rolled merge getYmlProjectData/mergeYmlProjectData read and deepmerge()'d project.yml files directly - a merge the extension owned in parallel to Ceedling's own, with a documented quirk (empty file resolves undefined, not {}) and no awareness of plugin defaults. Replaced with loadResolvedConfig(), which runs `ceedling dumpconfig` (through the same buildProjectArgs flags as any other invocation, so it reflects mixins too) into a throwaway temp file and parses that back. dumpconfig only writes to a file, never stdout. Resolved once per discover() cycle into a new resolvedConfig cache, reused by checkYmlProjectData and compileForDebug - both used to do their own independent parse of the same files. Net effect is one dumpconfig invocation per project per cycle in place of three separate file parses. The existing project.yml file watch already forces a fresh discover() on any config change, which is all the invalidation this cache needs. checkYmlProjectData's messages follow the new reality: the "couldn't load" message names the real ceedlingExplorer.projects setting (it previously named a setting that doesn't exist), and the plugin-not-enabled message no longer tells anyone to hand-edit project.yml - the bundled mixin makes that plugin's absence a sign the mixin itself failed, not a missing edit. deepmerge is no longer used anywhere; dropped from package.json. Co-Authored-By: Claude Sonnet 5 --- package-lock.json | 10 --- package.json | 1 - src/ceedlingEngine.ts | 123 ++++++++++++--------------- src/ceedlingMixinArgs.ts | 5 +- tests/unit/ceedlingEngine.test.ts | 134 +++++++++++++----------------- 5 files changed, 114 insertions(+), 159 deletions(-) diff --git a/package-lock.json b/package-lock.json index 97589be..ea3bbde 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13,7 +13,6 @@ "@types/semver": "^7.5.4", "@types/xml2js": "^0.4.3", "async-mutex": "^0.1.3", - "deepmerge": "^4.3.1", "js-yaml": "^3.13.1", "semver": "^7.5.4", "strip-ansi": "^6.0.0", @@ -2207,15 +2206,6 @@ "node": ">=4.0.0" } }, - "node_modules/deepmerge": { - "version": "4.3.1", - "resolved": "https://registry.npmjs.org/deepmerge/-/deepmerge-4.3.1.tgz", - "integrity": "sha512-3sUqbMEc77XqpdNO7FRyRog+eW3ph+GYCbj+rK+uYyRMuwsVy0rMiVtPn+QJlKFvWP/1PYpapqYn0Me2knFn+A==", - "license": "MIT", - "engines": { - "node": ">=0.10.0" - } - }, "node_modules/default-browser": { "version": "5.5.0", "resolved": "https://registry.npmjs.org/default-browser/-/default-browser-5.5.0.tgz", diff --git a/package.json b/package.json index 7c31d20..8d2107f 100644 --- a/package.json +++ b/package.json @@ -60,7 +60,6 @@ "@types/semver": "^7.5.4", "@types/xml2js": "^0.4.3", "async-mutex": "^0.1.3", - "deepmerge": "^4.3.1", "js-yaml": "^3.13.1", "semver": "^7.5.4", "strip-ansi": "^6.0.0", diff --git a/src/ceedlingEngine.ts b/src/ceedlingEngine.ts index 8998b99..abd6cfd 100644 --- a/src/ceedlingEngine.ts +++ b/src/ceedlingEngine.ts @@ -1,8 +1,8 @@ import { Mutex } from 'async-mutex'; import child_process from 'child_process'; -import deepmerge from 'deepmerge'; import fs from 'fs'; import yaml from 'js-yaml'; +import os from 'os'; import path from 'path'; import semver from 'semver'; import stripAnsi from 'strip-ansi'; @@ -26,7 +26,7 @@ import { testCaseFilterMatchedExactly, } from './ceedlingOutputParsing'; import { getTestListFromXmlReport } from './ceedlingXmlReport'; -import { buildProjectArgs } from './ceedlingMixinArgs'; +import { buildProjectArgs, quoteIfNeeded } from './ceedlingMixinArgs'; // Type-only, same reasoning as the `vscode` import above: Logger and ProblemMatcher both have // real runtime vscode calls of their own (an output channel, a DiagnosticCollection). Importing // only their types keeps this file from pulling either module's code in at require-time. Callers @@ -135,6 +135,8 @@ export class CeedlingEngine { private isPrettyTestFileLabelEnable: boolean = false; private projectData: Record = {}; + // Populated once per discover() cycle - see the comment in discover() itself. + private resolvedConfig: Record = {}; readonly problemMatcher: ProblemMatcher; @@ -196,6 +198,12 @@ export class CeedlingEngine { // not an error - see loadProjectPaths(). async discover(): Promise { this.ceedlingVersionChecked = false; + // Cleared here, not per-call: loadResolvedConfig spins up a real `ceedling dumpconfig` + // process, so each project's config is resolved once per discover() cycle and reused by + // every call within it (checkYmlProjectData below, and compileForDebug later). The + // existing project.yml file watch already forces a fresh discover() on any config change, + // which is all the invalidation this cache needs. + this.resolvedConfig = {}; this.loadProjectPaths(); const projectKeys = this.getExistingProjectKeys(); @@ -207,13 +215,14 @@ export class CeedlingEngine { await this.checkCeedlingVersion(); for (const projectKey of projectKeys) { - const ymlProjectData = await this.getYmlProjectData(projectKey); - this.logger.debug(`discover(projectKey=${projectKey}, ymlProjectData=${util.format(ymlProjectData)})`); - this.setBuildDirectory(projectKey, ymlProjectData); - this.setXmlReportPath(projectKey, ymlProjectData); - this.setFunctionRegex(projectKey, ymlProjectData); - this.setFileLabelRegex(projectKey, ymlProjectData); - this.setTestLabelRegex(projectKey, ymlProjectData); + const resolvedConfig = await this.loadResolvedConfig(projectKey); + this.resolvedConfig[projectKey] = resolvedConfig; + this.logger.debug(`discover(projectKey=${projectKey}, resolvedConfig=${util.format(resolvedConfig)})`); + this.setBuildDirectory(projectKey, resolvedConfig); + this.setXmlReportPath(projectKey, resolvedConfig); + this.setFunctionRegex(projectKey, resolvedConfig); + this.setFileLabelRegex(projectKey, resolvedConfig); + this.setTestLabelRegex(projectKey, resolvedConfig); } const errorMessage = await this.sanityCheck(projectKeys); @@ -381,8 +390,9 @@ export class CeedlingEngine { reporter.failed(file.id, `${result.stdout}\n${result.stderr}`); return undefined; } - const ymlProjectData = await this.getYmlProjectData(projectKey); - const ext = this.getExecutableExtension(ymlProjectData); + // Reads the same resolvedConfig cache discover() populated for this project - debugging + // is only reachable after a file has already been discovered, so it's already there. + const ext = this.getExecutableExtension(this.resolvedConfig[projectKey]); const testFileName = `${/([^/]*).c$/.exec(file.id)![1]}`; return `${testFileName}/${testFileName}${ext}`; } @@ -441,20 +451,26 @@ export class CeedlingEngine { } } + // Reads the config discover() already resolved this cycle - see resolvedConfig's own comment. + // A missing entry here means loadResolvedConfig() itself failed (dumpconfig errored, or its + // output couldn't be read/parsed), not that a mixin didn't apply. private async checkYmlProjectData(projectKey: string): Promise { - const ymlProjectData = await this.getYmlProjectData(projectKey) - if (!ymlProjectData) { - return `Failed to find or load the project.yml file for ${projectKey}. ` + - `Please check the ceedlingExplorer.projectPath option.`; + const resolvedConfig = this.resolvedConfig[projectKey]; + if (!resolvedConfig) { + return `Failed to resolve Ceedling's configuration for ${projectKey} via 'ceedling dumpconfig'. ` + + `Please check the ceedlingExplorer.projects option.`; } try { - if (!ymlProjectData[':plugins'][':enabled'].includes('report_tests_log_factory')) { + if (!resolvedConfig[':plugins'][':enabled'].includes('report_tests_log_factory')) { throw 'Report tests log factory plugin not enabled'; } } catch (e) { - return `The required Ceedling plugin 'report_tests_log_factory' is not enabled. ` + - `You have to edit ${this.getYmlProjectPath(projectKey)} file to enable the plugin.\n` + - `see https://github.com/ThrowTheSwitch/Ceedling/blob/master/plugins/report_tests_log_factory/README.md`; + // The bundled mixin (assets/mixins/report-tests-log-factory.yml) always enables this + // plugin, so reaching here means that mixin itself didn't take effect - not that the + // user needs to edit anything by hand. + return `The required Ceedling plugin 'report_tests_log_factory' did not take effect for ${projectKey}, ` + + `even though this extension's own mixin always enables it. Check that 'ceedling dumpconfig' ` + + `ran successfully and that ceedlingExplorer.mixins/projects[].mixins don't reference an invalid mixin.`; } } @@ -839,63 +855,34 @@ export class CeedlingEngine { ); } - private getYmlProjectData(projectKey: string): Promise { - try { - if (this.projectData[projectKey].ymlFileName != 'project.yml') { - return this.mergeYmlProjectData(projectKey); - } - return new Promise((resolve) => { - const project_yml = this.getYmlProjectPath(projectKey); - this.readFile(project_yml, 'utf8', (error, data) => { - if (error) { - this.logger.error(`Failed to read YAML file '${project_yml}': ${util.format(error)}`); - resolve(undefined); - return; - } - try { - resolve(yaml.safeLoad(data)); - } catch (e) { - this.logger.error(`Failed to parse YAML file '${project_yml}': ${util.format(e)}`); - resolve(undefined); - } - }); - }); - } - catch (e) { - this.logger.error(`getYmlProjectData()=${util.format(e)}`); + // Resolves a project's fully-merged Ceedling configuration via `ceedling dumpconfig`, rather + // than parsing project.yml (and its mixin, if any) by hand. dumpconfig applies Ceedling's own + // multi-file overlay, plugin defaults, and mixin-merge rules - see buildProjectArgs - so this + // reflects exactly what a real build would see, not just what's literally typed in one file. + // dumpconfig only writes to a file, never stdout, so this drives it through a throwaway + // temp file and reads that back. + private async loadResolvedConfig(projectKey: string): Promise { + const tmpPath = path.join( + os.tmpdir(), + `ceedling-dumpconfig-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}.yml`, + ); + const result = await this.execCeedling(['dumpconfig', quoteIfNeeded(tmpPath)], projectKey); + if (result.error) { + this.logger.error(`loadResolvedConfig(${projectKey}): 'ceedling dumpconfig' failed: ${util.format(result)}`); + return undefined; } - return Promise.resolve(undefined); - } - - private mergeYmlProjectData(projectKey: string): Promise { return new Promise((resolve) => { - const project_data = this.getYmlProjectPath(projectKey); - this.readFile(project_data, 'utf8', (error, data) => { + this.readFile(tmpPath, 'utf8', (error, data) => { if (error) { - this.logger.error(`Failed to read YAML file: ${util.format(error)}`); + this.logger.error(`loadResolvedConfig(${projectKey}): failed to read dumpconfig output '${tmpPath}': ${util.format(error)}`); resolve(undefined); return; } + fs.unlink(tmpPath, () => { }); // best-effort cleanup; a leftover temp file is harmless try { - const result = yaml.safeLoad(data); - const defaultYmlPath = path.resolve(this.projectData[projectKey].absPath, 'project.yml'); - this.readFile(defaultYmlPath, 'utf8', (error, data) => { - if (error) { - this.logger.error(`Failed to read default YAML file: ${util.format(error)}`); - resolve(result); - return; - } - try { - const defaultResult = yaml.safeLoad(data) || {}; - const mergedResult = deepmerge(defaultResult as object, (result as object) || {}); - resolve(mergedResult); - } catch (e) { - this.logger.error(`Failed to parse default YAML file: ${util.format(e)}`); - resolve(result); - } - }); + resolve(yaml.safeLoad(data)); } catch (e) { - this.logger.error(`Failed to parse YAML file: ${util.format(e)}`); + this.logger.error(`loadResolvedConfig(${projectKey}): failed to parse dumpconfig output: ${util.format(e)}`); resolve(undefined); } }); diff --git a/src/ceedlingMixinArgs.ts b/src/ceedlingMixinArgs.ts index 9658433..4f22e19 100644 --- a/src/ceedlingMixinArgs.ts +++ b/src/ceedlingMixinArgs.ts @@ -1,8 +1,9 @@ // Wraps a token in double quotes when it contains whitespace, so a path with spaces - routinely // true of a Windows install path or the OS temp directory - survives being joined into a single // shell command string by execCeedling. Every other value here is passed through unquoted, same -// as the literal flags around it. -function quoteIfNeeded(token: string): string { +// as the literal flags around it. Exported: execCeedling's dumpconfig output path needs the same +// treatment and isn't a --mixin flag itself. +export function quoteIfNeeded(token: string): string { return /\s/.test(token) ? `"${token}"` : token; } diff --git a/tests/unit/ceedlingEngine.test.ts b/tests/unit/ceedlingEngine.test.ts index c59a6c6..b653ac5 100644 --- a/tests/unit/ceedlingEngine.test.ts +++ b/tests/unit/ceedlingEngine.test.ts @@ -41,17 +41,6 @@ function fakeProblemMatcher(): ProblemMatcher { } as unknown as ProblemMatcher; } -// Keyed by absolute path. A missing key simulates an ENOENT read failure. -function fakeReadFile(files: Record): typeof fs.readFile { - return ((filePath: string, _encoding: any, callback: any) => { - if (Object.prototype.hasOwnProperty.call(files, filePath)) { - callback(null, files[filePath]); - } else { - callback(Object.assign(new Error(`ENOENT: no such file, open '${filePath}'`), { code: 'ENOENT' }), undefined); - } - }) as unknown as typeof fs.readFile; -} - // Records every invocation instead of actually spawning anything; resolves each call // with an empty successful result unless a call-specific result is queued. function fakeExec(calls: { command: string, options: any }[]): typeof child_process.exec { @@ -93,103 +82,92 @@ function withProject(engine: CeedlingEngine, key: string, data: { absPath: strin }; } -suite('getYmlProjectData (project.yml only, no mixin)', () => { - test('parses the file and resolves the plain object', async () => { - const engine = makeEngine({ - readFile: fakeReadFile({ '/ws/proj/project.yml': ':project:\n :build_root: build\n' }), - }); - withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); - const result = await (engine as any).getYmlProjectData('proj'); - assert.deepStrictEqual(result, { ':project': { ':build_root': 'build' } }); - }); +// Matches readFile calls by pattern rather than exact path: loadResolvedConfig's dumpconfig +// output path is generated fresh (pid/timestamp/random) on every call, not a fixed name. +function fakeReadFileMatching(pattern: RegExp, content: string | undefined): typeof fs.readFile { + return ((filePath: string, _encoding: any, callback: any) => { + if (content !== undefined && pattern.test(filePath)) { + callback(null, content); + } else { + callback(Object.assign(new Error(`ENOENT: no such file, open '${filePath}'`), { code: 'ENOENT' }), undefined); + } + }) as unknown as typeof fs.readFile; +} - test('resolves undefined when the file cannot be read', async () => { - const engine = makeEngine({ readFile: fakeReadFile({}) }); - withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); - const result = await (engine as any).getYmlProjectData('proj'); - assert.strictEqual(result, undefined); - }); +const DUMPCONFIG_OUTPUT_PATTERN = /ceedling-dumpconfig-.*\.yml$/; - test('resolves undefined when the file fails to parse', async () => { +suite('loadResolvedConfig (backed by `ceedling dumpconfig`)', () => { + test('runs dumpconfig with a .yml output path and parses what it wrote', async () => { + const calls: { command: string, options: any }[] = []; const engine = makeEngine({ - readFile: fakeReadFile({ '/ws/proj/project.yml': ':project: [\n broken' }), + exec: fakeExec(calls), + readFile: fakeReadFileMatching(DUMPCONFIG_OUTPUT_PATTERN, ':project:\n :build_root: from_dumpconfig\n'), }); withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); - const result = await (engine as any).getYmlProjectData('proj'); - assert.strictEqual(result, undefined); - }); - - // Current quirk, not a design choice: an empty project.yml parses to `undefined` (js-yaml's - // safeLoad on empty input), and nothing here guards it back to `{}` on this code path. - test('quirk: an empty project.yml resolves undefined, not {}', async () => { - const engine = makeEngine({ readFile: fakeReadFile({ '/ws/proj/project.yml': '' }) }); - withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); - const result = await (engine as any).getYmlProjectData('proj'); - assert.strictEqual(result, undefined); + (engine as any).ceedlingVersionChecked = true; + const result = await (engine as any).loadResolvedConfig('proj'); + assert.match(calls[0].command, /^ceedling dumpconfig \S*\.yml /); + assert.deepStrictEqual(result, { ':project': { ':build_root': 'from_dumpconfig' } }); }); -}); -suite('getYmlProjectData -> mergeYmlProjectData (project points at an alternate/mixin yml)', () => { - test('deep-merges the mixin over the base project.yml, mixin winning on scalar conflicts', async () => { + test('resolves undefined when dumpconfig itself reports an error', async () => { const engine = makeEngine({ - readFile: fakeReadFile({ - '/ws/proj/mixin.yml': ':project:\n :build_root: mixin_build\n:plugins:\n :enabled:\n - mixin_plugin\n', - '/ws/proj/project.yml': ':project:\n :build_root: base_build\n :test_file_prefix: test_\n:plugins:\n :enabled:\n - base_plugin\n', - }), + exec: ((command: string, options: any, callback: any) => { + callback(new Error('ceedling: command not found'), '', 'ceedling: command not found'); + return {} as child_process.ChildProcess; + }) as unknown as typeof child_process.exec, + readFile: fakeReadFileMatching(DUMPCONFIG_OUTPUT_PATTERN, ':project: {}\n'), }); - withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'mixin.yml' }); - const result = await (engine as any).getYmlProjectData('proj'); - // Scalar: mixin's value replaces the base's. - assert.strictEqual(result[':project'][':build_root'], 'mixin_build'); - // Untouched base key survives the merge. - assert.strictEqual(result[':project'][':test_file_prefix'], 'test_'); - // Lists concatenate base-then-mixin; deepmerge does not de-duplicate or reorder. - assert.deepStrictEqual(result[':plugins'][':enabled'], ['base_plugin', 'mixin_plugin']); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + (engine as any).ceedlingVersionChecked = true; + const result = await (engine as any).loadResolvedConfig('proj'); + assert.strictEqual(result, undefined); }); - test('resolves the mixin alone when the base project.yml cannot be read', async () => { + test('resolves undefined when the dumpconfig output file cannot be read', async () => { const engine = makeEngine({ - readFile: fakeReadFile({ '/ws/proj/mixin.yml': ':project:\n :build_root: mixin_build\n' }), + exec: fakeExec([]), + readFile: fakeReadFileMatching(DUMPCONFIG_OUTPUT_PATTERN, undefined), }); - withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'mixin.yml' }); - const result = await (engine as any).getYmlProjectData('proj'); - assert.deepStrictEqual(result, { ':project': { ':build_root': 'mixin_build' } }); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + (engine as any).ceedlingVersionChecked = true; + const result = await (engine as any).loadResolvedConfig('proj'); + assert.strictEqual(result, undefined); }); - test('resolves undefined when the mixin itself cannot be read (base is never even attempted)', async () => { + test('resolves undefined when the dumpconfig output fails to parse', async () => { const engine = makeEngine({ - readFile: fakeReadFile({ '/ws/proj/project.yml': ':project:\n :build_root: base_build\n' }), + exec: fakeExec([]), + readFile: fakeReadFileMatching(DUMPCONFIG_OUTPUT_PATTERN, ':project: [\n broken'), }); - withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'mixin.yml' }); - const result = await (engine as any).getYmlProjectData('proj'); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + (engine as any).ceedlingVersionChecked = true; + const result = await (engine as any).loadResolvedConfig('proj'); assert.strictEqual(result, undefined); }); }); -suite('checkYmlProjectData', () => { - test('reports a load failure, referencing the settings name as it stands today (a known-wrong name)', async () => { - const engine = makeEngine({ readFile: fakeReadFile({}) }); +suite('checkYmlProjectData (reads the resolvedConfig cache populated during discover())', () => { + test('reports a load failure, naming the real ceedlingExplorer.projects setting', async () => { + const engine = makeEngine(); withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + (engine as any).resolvedConfig = { proj: undefined }; const error = await (engine as any).checkYmlProjectData('proj'); - // Characterizes the pre-existing bug: the real setting is `ceedlingExplorer.projects`, - // not `ceedlingExplorer.projectPath`. The upcoming rewrite corrects this message. - assert.match(error, /ceedlingExplorer\.projectPath/); + assert.match(error, /ceedlingExplorer\.projects/); }); - test('reports the plugin-not-enabled error when report_tests_log_factory is missing', async () => { - const engine = makeEngine({ - readFile: fakeReadFile({ '/ws/proj/project.yml': ':plugins:\n :enabled:\n - other_plugin\n' }), - }); + test('reports the plugin-not-enabled error when report_tests_log_factory is missing from the resolved config', async () => { + const engine = makeEngine(); withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + (engine as any).resolvedConfig = { proj: { ':plugins': { ':enabled': ['other_plugin'] } } }; const error = await (engine as any).checkYmlProjectData('proj'); - assert.match(error, /report_tests_log_factory.*is not enabled/); + assert.match(error, /report_tests_log_factory.*did not take effect/); }); - test('reports nothing when report_tests_log_factory is enabled', async () => { - const engine = makeEngine({ - readFile: fakeReadFile({ '/ws/proj/project.yml': ':plugins:\n :enabled:\n - report_tests_log_factory\n' }), - }); + test('reports nothing when report_tests_log_factory is enabled in the resolved config', async () => { + const engine = makeEngine(); withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + (engine as any).resolvedConfig = { proj: { ':plugins': { ':enabled': ['report_tests_log_factory'] } } }; const error = await (engine as any).checkYmlProjectData('proj'); assert.strictEqual(error, undefined); }); From d3dffa2f8e5b193a6ee9f3c454b85dc09524df6f Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Sat, 12 Sep 2026 14:46:50 -0400 Subject: [PATCH 05/10] Document dumpconfig-based config and the new mixin settings README: drop the Getting Started step to hand-enable report_tests_log_factory - it's automatic now. New Mixins section explains the mechanism and the extension's own always-highest-priority mixin. ceedlingExplorer.mixins and projects[].mixins documented in the options table and the projects properties list, including the absent-vs-empty fallback rule. CHANGELOG.md and docs/ReleaseNotes.md get an Unreleased entry describing the behavior change and the new settings, for the maintainer to fold into whatever version this ships as. docs/Development.md's manual testing checklist gains two items: running with report_tests_log_factory removed from project.yml (should be unaffected), and a configured ceedlingExplorer.mixins entry showing up in the invocation ahead of the bundled mixin. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 13 +++++++++++++ README.md | 17 ++++++++++++++--- docs/Development.md | 2 ++ docs/ReleaseNotes.md | 14 ++++++++++++++ 4 files changed, 43 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aba8397..d1f41bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,19 @@ This format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) --- +# [Unreleased] + +## ⚠️ Changed + +* Project configuration is now resolved with Ceedling’s own `ceedling dumpconfig` instead of the extension parsing and merging `project.yml` (and any mixin) itself. This reflects Ceedling’s actual merge — plugin defaults included — rather than a parallel one the extension maintained on its own. + +## 🌟 Added + +* The `report_tests_log_factory` plugin’s `cppunit` report — the XML report this extension depends on — is now enabled automatically via a bundled [mixin](README.md#mixins). Manually enabling it in `project.yml` is no longer required. +* New `ceedlingExplorer.mixins` and `ceedlingExplorer.projects[].mixins` settings expose Ceedling’s own mixin mechanism, so you can layer toolchain targets, CI overrides, or other configuration without hand-editing `project.yml`. See [Mixins](README.md#mixins). + +--- + # [2.0.1] — 2026-08-31 ## ⚠️ Changed diff --git a/README.md b/README.md index cc4254b..04114ff 100644 --- a/README.md +++ b/README.md @@ -53,11 +53,12 @@ Ceedling 1.0.0 also requires test-file preprocessing disabled (`:project` ↳ `: * Open the workspace or folder containing your Ceedling project. * Configure your Ceedling project configuration filepath in VS Code’s settings if required [see below](#options). * Configure the shell path where Ceedling is installed in VS Code’s settings if required (Windows) [see below](#options). -* [Enable and configure][cppunit-plugin] the `report_tests_log_factory` Ceedling plugin with the `cppunit` option in your Ceedling project configuration. This generates an XML test report on which this extension depends. * Open the Testing view. * Run your tests using the run/debug icons in VS Code’s _Testing_ view or in your test file’s gutter. -[cppunit-plugin]: https://throwtheswitch.github.io/Ceedling/1.1.0/plugins/report-tests-log-factory/#configuration +The extension resolves your project’s configuration with [`ceedling dumpconfig`][dumpconfig-docs] and always applies its own [mixin](#mixins) enabling the `report_tests_log_factory` plugin’s `cppunit` report — the XML report this extension depends on. No manual plugin setup is required. + +[dumpconfig-docs]: https://throwtheswitch.github.io/Ceedling/1.1.0/getting-started/command-line/#ceedling-dumpconfig-filepath-sections # Running and debugging tests @@ -75,7 +76,8 @@ This table of options should be read as `ceedlingExplorer.`. | `ceedlingExplorer` Property | Description | |---|---| -| `.projects` | An array of objects with the path to the Ceedling project (or yml-file) to use (relative to the workspace folder). See below for its `path`, `debugLaunchConfig`, and `name` properties. | +| `.projects` | An array of objects with the path to the Ceedling project (or yml-file) to use (relative to the workspace folder). See below for its `path`, `debugLaunchConfig`, `name`, and `mixins` properties. | +| `.mixins` | An array of [Ceedling mixins](#mixins) — names, built-in toolchain targets (e.g. `gcc`, `clang`), or file paths — applied to every project that doesn’t define its own `mixins` (see `.projects` below), lowest to highest priority. By default it is `[]`. | | `.shellPath` | The path to the shell where Ceedling is installed. By default (or if this option is set to `null`), it uses the OS default shell. | | `.prettyTestLabel` | Shortens the test label in the _Testing_ explorer by dropping its leading prefix (e.g. inactive `test_BlinkTaskShouldToggleLed` vs. active `BlinkTaskShouldToggleLed`).

Inactive:
prettyTestLabelInactive

Active:
prettyTestLabelActive| | `.prettyTestFileLabel` | Shortens the test file label in the _Testing_ explorer by dropping its path, leading prefix, and file type (e.g. inactive `test/LEDs/test_BlinkTask.c` vs. active `BlinkTask`).

Inactive:
prettyTestFileLabelInactive

Active:
prettyTestFileLabelActive | @@ -90,6 +92,15 @@ This table of options should be read as `ceedlingExplorer.`. - `path`: can point either to a directory containing a "project.yml" file or directly to another .yml file (with the respective project.yml in the same directory). This path should be relative to the workspace root directory. - `debugLaunchConfig`: must be the *name* property of the launch config (launch.json) that is used for this project. The `${command:ceedlingExplorer.debugTestExecutable}` must still be used. - `name` (optional): used as the name for the folder containing the tests in the test explorer. +- `mixins` (optional): this project’s own [mixins](#mixins), lowest to highest priority. When omitted entirely, `ceedlingExplorer.mixins` is used instead; set to `[]` explicitly to opt this project out of that workspace-wide list rather than falling back to it. + +## Mixins + +A [Ceedling mixin][mixins-docs] is a YAML fragment merged into a project’s configuration at build time — useful for toolchain targets, CI-specific overrides, or any configuration you don’t want duplicated into every `project.yml`. Configure them with `ceedlingExplorer.mixins` and `ceedlingExplorer.projects[].mixins` (see [Options](#options)): later entries win on conflicts, matching Ceedling’s own `--mixin` precedence. + +The extension always applies one more mixin after any you configure: it enables the `report_tests_log_factory` plugin’s `cppunit` report, which the Testing view depends on to read results. This mixin is not configurable and always has the highest priority, so nothing you set here can disable test reporting — even if you also enable `report_tests_log_factory` yourself, applying it twice is harmless. + +[mixins-docs]: https://throwtheswitch.github.io/Ceedling/1.1.0/configuration/mixins/ ## Problem matching diff --git a/docs/Development.md b/docs/Development.md index dc87134..9a07ab7 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -139,6 +139,8 @@ All steps run against [`tests/manual/`](../tests/manual/), opened as its own fol - [ ] **Crash handling.** `test_should_crash` in `test_calculator_crash.c` shows as a failed test, at the exact crashing line, on both Ceedling 1.0.0 and 1.1.0. - [ ] **Crash log link.** With Ceedling 1.1.0 running on a platform that supports `gdb`, set `:use_backtrace:` ⇒ `:gdb` in `project.yml` and re-run `test_should_crash`. Its failure message ends with a clickable link to the gdb log file. Revert the setting after. - [ ] **Diagnostics don’t go stale.** Introduce a syntax error, run just `test_add_should_ReturnSum` alone from its gutter icon. Confirm the error appears in the Problems panel. Fix the error, then run the whole file (not that same test again). Confirm the Problems panel entry is gone. +- [ ] **No manual plugin setup.** Comment out `report_tests_log_factory` from `:plugins:` ↳ `:enabled:` in `tests/manual/project.yml`. Refresh, run all. Discovery and results are unaffected — the bundled mixin still enables the plugin. Revert the edit after. +- [ ] **Configured mixin.** Add `"ceedlingExplorer.mixins": ["gcc"]` (or another [built-in toolchain target](https://throwtheswitch.github.io/Ceedling/1.1.0/configuration/mixins/#built-in-toolchain-mixins)) to `tests/manual/.vscode/settings.json`. Refresh, run all. The run still succeeds; the Ceedling Explorer log (Trace level) shows the extra `--mixin` flag in the invocation, before the extension’s own bundled mixin. Revert the setting after. ## 5. The Sidecar diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index cf26d6b..ca0f8d0 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -4,6 +4,20 @@ Complemented by three other documents: [../CHANGELOG.md](../CHANGELOG.md), [Know --- +# Unreleased + +## 👀 Highlights + +Project configuration is now resolved with Ceedling’s own `ceedling dumpconfig` command instead of the extension parsing and merging `project.yml` by hand — the same merge a real build sees, plugin defaults included. + +The `report_tests_log_factory` plugin’s `cppunit` report, which the Testing view depends on to read results, is now enabled automatically via a mixin bundled with the extension. Manually editing `project.yml` to enable it is no longer necessary. New `ceedlingExplorer.mixins` and `ceedlingExplorer.projects[].mixins` settings expose Ceedling’s own mixin mechanism directly, for anyone who wants to layer toolchain targets, CI overrides, or other configuration. See [Mixins](../README.md#mixins). + +## 🚨 Important Changes + +None — existing `project.yml` files that already enable `report_tests_log_factory` themselves continue to work unchanged; the extension’s mixin is additive. + +--- + # 2.0.0 — 2026-08-31 ## 👀 Highlights From adab063d5a851d93fb5ba1af89483f55501d1d87 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Sat, 12 Sep 2026 15:01:06 -0400 Subject: [PATCH 06/10] Ground loadResolvedConfig's tests in real `ceedling dumpconfig` output Added two captured dumpconfig fixtures (tests/fixtures/dumpconfig/) against a copy of tests/manual, with and without the bundled report-plugin mixin applied, plus a test suite exercising checkYmlProjectData/ setBuildDirectory/setXmlReportPath against them directly, alongside the existing synthetic-YAML tests. First capture was against a Ceedling 1.2.0 development checkout and turned up custom !ruby/object:FilenameExtension tags under :extension: that js-yaml's safe loader can't parse - an unreleased regression in dumpconfig's own implementation (dump_yaml dumps the live internal config object graph via YAML.dump rather than a plain copy), confirmed against Ceedling 1.1.8 (clean, plain YAML) and reported upstream for a fix ahead of 1.2.0. Recaptured both fixtures against 1.1.8; the design needs no workaround. Full provenance and detail in tests/fixtures/dumpconfig/README.md. Also confirmed, by hand, with real Ceedling 1.1.8, that a stripped tests/manual (report_tests_log_factory removed entirely from project.yml) running `ceedling test:all --mixin assets/mixins/report-tests-log-factory.yml` alone still produces build/artifacts/test/cppunit_tests_report.xml. Co-Authored-By: Claude Sonnet 5 --- tests/fixtures/dumpconfig/README.md | 25 +++ .../manual-project-no-report-plugin.yml | 188 +++++++++++++++++ .../manual-project-with-bundled-mixin.yml | 198 ++++++++++++++++++ tests/unit/ceedlingEngine.test.ts | 47 ++++- 4 files changed, 454 insertions(+), 4 deletions(-) create mode 100644 tests/fixtures/dumpconfig/README.md create mode 100644 tests/fixtures/dumpconfig/manual-project-no-report-plugin.yml create mode 100644 tests/fixtures/dumpconfig/manual-project-with-bundled-mixin.yml diff --git a/tests/fixtures/dumpconfig/README.md b/tests/fixtures/dumpconfig/README.md new file mode 100644 index 0000000..b617205 --- /dev/null +++ b/tests/fixtures/dumpconfig/README.md @@ -0,0 +1,25 @@ +# `dumpconfig` fixtures + +Real `ceedling dumpconfig` output, captured against a copy of +[`tests/manual`](../../manual), with `report_tests_log_factory` removed from its +`project.yml`. Captured with Ceedling **1.1.8** (a released version within this +extension's supported range). + +An earlier capture used a Ceedling 1.2.0 development/prerelease checkout and turned +up custom `!ruby/object:FilenameExtension` tags under `:extension:` that `js-yaml`'s +safe loader refuses to parse. That's an unreleased regression in `dumpconfig` +itself — `configurator_builder.rb`'s `normalize_filename_extensions` wraps every +`:extension:` sub-key in a `FilenameExtension` value object for the build's own use, +and `dump_yaml` dumps that live object graph directly via `YAML.dump` instead of a +plain, portable copy. Confirmed absent on 1.1.8: `:extension:` values are plain +strings there, as expected. Will be fixed upstream before 1.2.0 ships. + +- `manual-project-no-report-plugin.yml` — `ceedling dumpconfig` with no `--mixin`. + `:plugins: :enabled:` has no `report_tests_log_factory`. +- `manual-project-with-bundled-mixin.yml` — the same project, `--mixin + assets/mixins/report-tests-log-factory.yml` added. `:plugins: :enabled:` now + includes `report_tests_log_factory`, and `:report_tests_log_factory: :reports:` + includes `cppunit`. A full `ceedling test:all` with this same mixin was also run by + hand (Ceedling 1.1.8) against the stripped project and did produce + `build/artifacts/test/cppunit_tests_report.xml`, confirming the mixin is + sufficient on its own, with no other change to `project.yml`. diff --git a/tests/fixtures/dumpconfig/manual-project-no-report-plugin.yml b/tests/fixtures/dumpconfig/manual-project-no-report-plugin.yml new file mode 100644 index 0000000..e5e695a --- /dev/null +++ b/tests/fixtures/dumpconfig/manual-project-no-report-plugin.yml @@ -0,0 +1,188 @@ +--- +:project: + :name: VSCode-Ceedling Manual Test Project + :which_ceedling: gem + :ceedling_version: 1.1.0 + :use_mocks: true + :use_test_preprocessor: :none + :use_backtrace: :simple + :build_root: build + :test_file_prefix: test_ + :default_tasks: + - test:all + :test_threads: :auto + :compile_threads: :auto + :use_exceptions: false + :use_partials: false + :release_build: false +:plugins: + :load_paths: + - "/private/tmp/ceedling-1.1.8/plugins" + :enabled: + - report_tests_pretty_stdout + :display_raw_test_results: false + :report_tests_pretty_stdout_path: "/private/tmp/ceedling-1.1.8/plugins/report_tests_pretty_stdout" +:paths: + :test: + - "+:test/**" + - "-:test/support" + :source: + - src/** + :include: + - src/** + :support: + - test/support + :libraries: [] + :test_toolchain_include: [] + :release_toolchain_include: [] +:defines: + :test: + - TEST + :use_test_definition: false + :release: [] +:cmock: + :plugins: + - :ignore + - :callback + :verbosity: 2 + :when_no_prototypes: :warn + :skeleton_path: '' + :mock_prefix: mock_ + :mock_suffix: '' + :strippables: + - "(?:__attribute__\\s*\\([ (]*.*?[ )]*\\)+)" + :attributes: + - __ramfunc + - __irq + - __fiq + - register + - extern + :c_calling_conventions: + - __stdcall + - __cdecl + - __fastcall + :treat_externs: :exclude + :treat_inlines: :exclude + :treat_as: + uint8: HEX8 + uint16: HEX16 + uint32: UINT32 + int8: INT8 + bool: UINT8 + :memcmp_if_unknown: true + :when_ptr: :compare_data + :weak: '' + :enforce_strict_ordering: true + :fail_on_unexpected_calls: true + :callback_include_count: true + :callback_after_arg_check: false + :exclude_setjmp_h: false + :vendor_path: "/private/tmp/ceedling-1.1.8/vendor" + :includes: [] + :defines: + - CMOCK_MOCK_PREFIX=mock_ + :unity_helper_path: [] + :mock_path: build/test/mocks +:unity: + :defines: + - UNITY_EXCLUDE_FLOAT + - UNITY_SUPPORT_TEST_CASES + - UNITY_SUPPORT_VARIADIC_MACROS + :use_param_tests: true + :vendor_path: "/private/tmp/ceedling-1.1.8/vendor" +:history: + :config: + - :type: :file + :path: "./project.yml" + :mechanism: :project +:cexception: + :vendor_path: "/private/tmp/ceedling-1.1.8/vendor" + :defines: [] +:release_build: + :use_assembly: false + :artifacts: [] +:test_build: + :use_assembly: false + :preprocess_force_fallback: false + :preprocess_directives_only_available: false +:partials: + :max_extraction_length: 5000 +:environment: [] +:files: + :test: [] + :source: [] + :assembly: [] + :support: [] + :include: [] +:flags: {} +:libraries: + :flag: "-l${1}" + :path_flag: "-L ${1}" + :test: [] + :release: [] +:extension: + :header: ".h" + :source: ".c" + :assembly: ".s" + :object: ".o" + :libraries: + - ".a" + - ".so" + :executable: ".out" + :map: ".map" + :list: ".lst" + :testpass: ".pass" + :testfail: ".fail" + :dependencies: ".d" + :yaml: ".yml" +:test_runner: + :cmdline_args: true + :includes: [] + :defines: + - UNITY_EXCLUDE_FLOAT + - UNITY_SUPPORT_TEST_CASES + - UNITY_SUPPORT_VARIADIC_MACROS + :file_suffix: _runner + :mock_prefix: mock_ + :mock_suffix: '' + :enforce_strict_ordering: true + :use_param_tests: true +:tools: + :test_compiler: + :executable: gcc + :name: default_test_compiler + :optional: false + :arguments: + - -I"${5}" + - -D"${6}" + - "-DGNU_COMPILER" + - "-g" + - -c "${1}" + - -o "${2}" + - "-MMD" + - -MF "${4}" + :stderr_redirect: :none + :test_linker: + :executable: gcc + :name: default_test_linker + :optional: false + :arguments: + - "${1}" + - "${5}" + - -o "${2}" + - '' + - "${4}" + :stderr_redirect: :none + :test_fixture: + :executable: "${1}" + :name: default_test_fixture + :optional: false + :arguments: [] + :stderr_redirect: :none + :test_fixture_simple_backtrace: + :executable: "${1}" + :name: default_test_fixture_simple_backtrace + :optional: false + :arguments: + - "${2}" + :stderr_redirect: :none diff --git a/tests/fixtures/dumpconfig/manual-project-with-bundled-mixin.yml b/tests/fixtures/dumpconfig/manual-project-with-bundled-mixin.yml new file mode 100644 index 0000000..a7ea8c8 --- /dev/null +++ b/tests/fixtures/dumpconfig/manual-project-with-bundled-mixin.yml @@ -0,0 +1,198 @@ +--- +:project: + :name: VSCode-Ceedling Manual Test Project + :which_ceedling: gem + :ceedling_version: 1.1.0 + :use_mocks: true + :use_test_preprocessor: :none + :use_backtrace: :simple + :build_root: build + :test_file_prefix: test_ + :default_tasks: + - test:all + :test_threads: :auto + :compile_threads: :auto + :use_exceptions: false + :use_partials: false + :release_build: false +:plugins: + :load_paths: + - "/private/tmp/ceedling-1.1.8/plugins" + :enabled: + - report_tests_log_factory + - report_tests_pretty_stdout + :display_raw_test_results: false + :report_tests_log_factory_path: "/private/tmp/ceedling-1.1.8/plugins/report_tests_log_factory" + :report_tests_pretty_stdout_path: "/private/tmp/ceedling-1.1.8/plugins/report_tests_pretty_stdout" +:paths: + :test: + - "+:test/**" + - "-:test/support" + :source: + - src/** + :include: + - src/** + :support: + - test/support + :libraries: [] + :test_toolchain_include: [] + :release_toolchain_include: [] +:defines: + :test: + - TEST + :use_test_definition: false + :release: [] +:cmock: + :plugins: + - :ignore + - :callback + :verbosity: 2 + :when_no_prototypes: :warn + :skeleton_path: '' + :mock_prefix: mock_ + :mock_suffix: '' + :strippables: + - "(?:__attribute__\\s*\\([ (]*.*?[ )]*\\)+)" + :attributes: + - __ramfunc + - __irq + - __fiq + - register + - extern + :c_calling_conventions: + - __stdcall + - __cdecl + - __fastcall + :treat_externs: :exclude + :treat_inlines: :exclude + :treat_as: + uint8: HEX8 + uint16: HEX16 + uint32: UINT32 + int8: INT8 + bool: UINT8 + :memcmp_if_unknown: true + :when_ptr: :compare_data + :weak: '' + :enforce_strict_ordering: true + :fail_on_unexpected_calls: true + :callback_include_count: true + :callback_after_arg_check: false + :exclude_setjmp_h: false + :vendor_path: "/private/tmp/ceedling-1.1.8/vendor" + :includes: [] + :defines: + - CMOCK_MOCK_PREFIX=mock_ + :unity_helper_path: [] + :mock_path: build/test/mocks +:unity: + :defines: + - UNITY_EXCLUDE_FLOAT + - UNITY_SUPPORT_TEST_CASES + - UNITY_SUPPORT_VARIADIC_MACROS + :use_param_tests: true + :vendor_path: "/private/tmp/ceedling-1.1.8/vendor" +:history: + :config: + - :type: :file + :path: "./project.yml" + :mechanism: :project + - :type: :file + :path: "/Users/mkarlesky/git/ThrowTheSwitch/VSCode-Ceedling/assets/mixins/report-tests-log-factory.yml" + :mechanism: :cmdline + :source: "--mixin" + :value: "/Users/mkarlesky/git/ThrowTheSwitch/VSCode-Ceedling/assets/mixins/report-tests-log-factory.yml" +:report_tests_log_factory: + :reports: + - cppunit +:cexception: + :vendor_path: "/private/tmp/ceedling-1.1.8/vendor" + :defines: [] +:release_build: + :use_assembly: false + :artifacts: [] +:test_build: + :use_assembly: false + :preprocess_force_fallback: false + :preprocess_directives_only_available: false +:partials: + :max_extraction_length: 5000 +:environment: [] +:files: + :test: [] + :source: [] + :assembly: [] + :support: [] + :include: [] +:flags: {} +:libraries: + :flag: "-l${1}" + :path_flag: "-L ${1}" + :test: [] + :release: [] +:extension: + :header: ".h" + :source: ".c" + :assembly: ".s" + :object: ".o" + :libraries: + - ".a" + - ".so" + :executable: ".out" + :map: ".map" + :list: ".lst" + :testpass: ".pass" + :testfail: ".fail" + :dependencies: ".d" + :yaml: ".yml" +:test_runner: + :cmdline_args: true + :includes: [] + :defines: + - UNITY_EXCLUDE_FLOAT + - UNITY_SUPPORT_TEST_CASES + - UNITY_SUPPORT_VARIADIC_MACROS + :file_suffix: _runner + :mock_prefix: mock_ + :mock_suffix: '' + :enforce_strict_ordering: true + :use_param_tests: true +:tools: + :test_compiler: + :executable: gcc + :name: default_test_compiler + :optional: false + :arguments: + - -I"${5}" + - -D"${6}" + - "-DGNU_COMPILER" + - "-g" + - -c "${1}" + - -o "${2}" + - "-MMD" + - -MF "${4}" + :stderr_redirect: :none + :test_linker: + :executable: gcc + :name: default_test_linker + :optional: false + :arguments: + - "${1}" + - "${5}" + - -o "${2}" + - '' + - "${4}" + :stderr_redirect: :none + :test_fixture: + :executable: "${1}" + :name: default_test_fixture + :optional: false + :arguments: [] + :stderr_redirect: :none + :test_fixture_simple_backtrace: + :executable: "${1}" + :name: default_test_fixture_simple_backtrace + :optional: false + :arguments: + - "${2}" + :stderr_redirect: :none diff --git a/tests/unit/ceedlingEngine.test.ts b/tests/unit/ceedlingEngine.test.ts index b653ac5..ab63b38 100644 --- a/tests/unit/ceedlingEngine.test.ts +++ b/tests/unit/ceedlingEngine.test.ts @@ -1,15 +1,18 @@ import * as assert from 'assert'; import * as child_process from 'child_process'; import * as fs from 'fs'; +import yaml from 'js-yaml'; +import * as path from 'path'; import type vscode from 'vscode'; import { CeedlingEngine } from '../../src/ceedlingEngine'; import type { Logger } from '../../src/logger'; import type { ProblemMatcher } from '../../src/problemMatcher'; -// Characterizes CeedlingEngine's current YAML-loading and command-construction behavior ahead of -// replacing it with `ceedling dumpconfig` (see the project plan). None of this is new behavior - -// every assertion here, including the ones that look like bugs, documents what the code already -// does, so the refactor has a baseline to diff against. +// Exercises CeedlingEngine's `ceedling dumpconfig`-backed config resolution and its +// --project/--mixin command construction, with child_process/fs/vscode all faked - see +// makeEngine()/withProject() below. A few tests instead replay real `ceedling dumpconfig` output +// captured against tests/manual (see tests/fixtures/dumpconfig/), to ground the faked-YAML tests +// against what Ceedling itself actually produces. function fakeWorkspaceFolder(fsPath: string): vscode.WorkspaceFolder { return { @@ -233,3 +236,39 @@ suite('execCeedling: --project/--mixin flag construction', () => { assert.match(devCalls[0].command, /--verbosity debug$/); }); }); + +const dumpconfigFixturesRoot = path.resolve(__dirname, '..', '..', '..', 'tests', 'fixtures', 'dumpconfig'); + +function loadDumpconfigFixture(name: string): any { + return yaml.safeLoad(fs.readFileSync(path.resolve(dumpconfigFixturesRoot, name), 'utf8')); +} + +suite('real `ceedling dumpconfig` output (captured against tests/manual, see tests/fixtures/dumpconfig/README.md)', () => { + test('without the bundled mixin, report_tests_log_factory is genuinely absent - checkYmlProjectData catches it', async () => { + const engine = makeEngine(); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + (engine as any).resolvedConfig = { proj: loadDumpconfigFixture('manual-project-no-report-plugin.yml') }; + const error = await (engine as any).checkYmlProjectData('proj'); + assert.match(error, /report_tests_log_factory.*did not take effect/); + }); + + test('with the bundled mixin applied, report_tests_log_factory and its cppunit report are present', async () => { + const engine = makeEngine(); + withProject(engine, 'proj', { absPath: '/ws/proj', ymlFileName: 'project.yml' }); + const resolvedConfig = loadDumpconfigFixture('manual-project-with-bundled-mixin.yml'); + (engine as any).resolvedConfig = { proj: resolvedConfig }; + const error = await (engine as any).checkYmlProjectData('proj'); + assert.strictEqual(error, undefined); + assert.deepStrictEqual(resolvedConfig[':report_tests_log_factory'][':reports'], ['cppunit']); + }); + + test('setBuildDirectory/setXmlReportPath read real dumpconfig output correctly', () => { + const engine = makeEngine(); + const resolvedConfig = loadDumpconfigFixture('manual-project-with-bundled-mixin.yml'); + (engine as any).setBuildDirectory('proj', resolvedConfig); + (engine as any).setXmlReportPath('proj', resolvedConfig); + assert.strictEqual((engine as any).buildDirectories['proj'], 'build'); + // No :cppunit: :filename: override in this real output - the plugin default applies. + assert.strictEqual((engine as any).reportFilenames['proj'], 'cppunit_tests_report.xml'); + }); +}); From f62098a536cfb1cb5c0a83ede56fdf2538de93c1 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Sat, 12 Sep 2026 15:12:32 -0400 Subject: [PATCH 07/10] Update manual fixture; enrich logging; warn on non-default report filename tests/manual/project.yml no longer enables report_tests_log_factory itself - it now demonstrates, permanently, that the bundled mixin alone is sufficient. Verified by hand against real Ceedling 1.1.8: a full `ceedling test:all --mixin assets/mixins/report-tests-log-factory.yml` run against this exact project.yml still produces build/artifacts/test/cppunit_tests_report.xml. docs/Development.md's checklist item updated to match - there's no longer an edit to make and revert, the project just works this way. Logging: loadProjectPaths' existing per-project debug line now includes the resolved mixins array, and loadResolvedConfig logs its dumpconfig invocation at debug level before running it - both previously invisible even at Trace verbosity. New user-facing warning: setXmlReportPath now calls the new Logger.showWarning (added alongside the existing showInfo/showError) when the resolved CppUnit report filename isn't Ceedling's default - project.yml or a configured mixin changed it. Shown once per project per activation, not once per discover() cycle, since the project.yml file watch re-runs discover() on every save. Co-Authored-By: Claude Sonnet 5 --- CHANGELOG.md | 1 + README.md | 2 + docs/Development.md | 2 +- src/ceedlingEngine.ts | 30 +++++++++++-- src/logger.ts | 6 +++ tests/manual/project.yml | 11 ++--- tests/unit/ceedlingEngine.test.ts | 71 ++++++++++++++++++++++++++++--- 7 files changed, 105 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1f41bf..d619f09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ This format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) * The `report_tests_log_factory` plugin’s `cppunit` report — the XML report this extension depends on — is now enabled automatically via a bundled [mixin](README.md#mixins). Manually enabling it in `project.yml` is no longer required. * New `ceedlingExplorer.mixins` and `ceedlingExplorer.projects[].mixins` settings expose Ceedling’s own mixin mechanism, so you can layer toolchain targets, CI overrides, or other configuration without hand-editing `project.yml`. See [Mixins](README.md#mixins). +* A one-time warning notification when `project.yml` or a configured mixin sets a non-default CppUnit XML report filename, naming the project and the resolved filename. --- diff --git a/README.md b/README.md index 04114ff..d49b46b 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,8 @@ A [Ceedling mixin][mixins-docs] is a YAML fragment merged into a project’s con The extension always applies one more mixin after any you configure: it enables the `report_tests_log_factory` plugin’s `cppunit` report, which the Testing view depends on to read results. This mixin is not configurable and always has the highest priority, so nothing you set here can disable test reporting — even if you also enable `report_tests_log_factory` yourself, applying it twice is harmless. +If `project.yml` or one of your own mixins sets `:report_tests_log_factory` ↳ `:cppunit` ↳ `:filename` to something other than Ceedling’s default, the extension shows a one-time warning naming the project and the resolved filename — it still uses that filename correctly, the warning only calls out that the report lives somewhere non-default in case that wasn’t intentional. + [mixins-docs]: https://throwtheswitch.github.io/Ceedling/1.1.0/configuration/mixins/ ## Problem matching diff --git a/docs/Development.md b/docs/Development.md index 9a07ab7..def925e 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -139,7 +139,7 @@ All steps run against [`tests/manual/`](../tests/manual/), opened as its own fol - [ ] **Crash handling.** `test_should_crash` in `test_calculator_crash.c` shows as a failed test, at the exact crashing line, on both Ceedling 1.0.0 and 1.1.0. - [ ] **Crash log link.** With Ceedling 1.1.0 running on a platform that supports `gdb`, set `:use_backtrace:` ⇒ `:gdb` in `project.yml` and re-run `test_should_crash`. Its failure message ends with a clickable link to the gdb log file. Revert the setting after. - [ ] **Diagnostics don’t go stale.** Introduce a syntax error, run just `test_add_should_ReturnSum` alone from its gutter icon. Confirm the error appears in the Problems panel. Fix the error, then run the whole file (not that same test again). Confirm the Problems panel entry is gone. -- [ ] **No manual plugin setup.** Comment out `report_tests_log_factory` from `:plugins:` ↳ `:enabled:` in `tests/manual/project.yml`. Refresh, run all. Discovery and results are unaffected — the bundled mixin still enables the plugin. Revert the edit after. +- [ ] **No manual plugin setup.** `tests/manual/project.yml` deliberately does not enable `report_tests_log_factory` itself. Discovery and results still work — the bundled mixin enables it. Confirms this on every run of this checklist, not just once. - [ ] **Configured mixin.** Add `"ceedlingExplorer.mixins": ["gcc"]` (or another [built-in toolchain target](https://throwtheswitch.github.io/Ceedling/1.1.0/configuration/mixins/#built-in-toolchain-mixins)) to `tests/manual/.vscode/settings.json`. Refresh, run all. The run still succeeds; the Ceedling Explorer log (Trace level) shows the extra `--mixin` flag in the invocation, before the extension’s own bundled mixin. Revert the setting after. ## 5. The Sidecar diff --git a/src/ceedlingEngine.ts b/src/ceedlingEngine.ts index abd6cfd..b3f5b61 100644 --- a/src/ceedlingEngine.ts +++ b/src/ceedlingEngine.ts @@ -36,6 +36,9 @@ import type { FileDiagnostic, ProblemMatcher, ProblemMatchingPattern } from './p const MINIMUM_CEEDLING_VERSION = '1.0.0'; const TEST_VERBOSITY_FLAG = '--verbosity debug'; +// Ceedling's own default for report_tests_log_factory's cppunit report. Also this extension's +// fallback when nothing in the resolved config overrides it - see setXmlReportPath. +const DEFAULT_CPPUNIT_REPORT_FILENAME = 'cppunit_tests_report.xml'; export type ProjectData = { projectPath: string, @@ -130,6 +133,10 @@ export class CeedlingEngine { private testLabelRegexps: Record = {}; private buildDirectories: Record = {}; private reportFilenames: Record = {}; + // Warned about at most once per project per activation, not once per discover() cycle - the + // project.yml file watch re-runs discover() on every save, and this would otherwise pop up a + // notification on every single one. + private warnedNonDefaultReportFilename: Set = new Set(); private isPrettyTestLabelEnable: boolean = false; private isPrettyTestFileLabelEnable: boolean = false; @@ -526,13 +533,17 @@ export class CeedlingEngine { if (!(fs.existsSync(absolutePath) && fs.lstatSync(absolutePath).isDirectory())) { throw new CeedlingLoadError(`The project path ${absolutePath} does not exist or is not a directory.`); } else { - this.logger.debug(`loadProjectPaths: resolved project '${key}' -> path='${absolutePath}', ymlFileName='${ymlName}'`); + const mixins = projectConfig.mixins !== undefined ? projectConfig.mixins : globalMixins; + this.logger.debug( + `loadProjectPaths: resolved project '${key}' -> path='${absolutePath}', ymlFileName='${ymlName}', ` + + `mixins=${util.format(mixins)}` + ); this.projectData[key] = { debugLaunchConfig: projectConfig.debugLaunchConfig, projectPath: projectConfig.path, ymlFileName: ymlName, absPath: absolutePath, - mixins: projectConfig.mixins !== undefined ? projectConfig.mixins : globalMixins, + mixins, files: {} }; } @@ -724,7 +735,7 @@ export class CeedlingEngine { } private setXmlReportPath(projectKey: string, ymlProjectData: any = undefined) { - let reportFilename = 'cppunit_tests_report.xml'; + let reportFilename = DEFAULT_CPPUNIT_REPORT_FILENAME; if (ymlProjectData) { try { const ymlProjectReportFilename = ymlProjectData[':report_tests_log_factory'][':cppunit'][':filename']; @@ -735,6 +746,18 @@ export class CeedlingEngine { this.logger.debug(`setXmlReportPath(${projectKey}): no ':report_tests_log_factory'/':cppunit'/':filename' in project.yml, using default '${reportFilename}': ${e}`); } } + // A mixin or project.yml setting a non-default filename changes where this extension looks + // for the report (getXmlReportPath), silently as far as the user is concerned - surface it + // once per project so an unexpected report location doesn't look like a mystery failure. + if (reportFilename !== DEFAULT_CPPUNIT_REPORT_FILENAME && !this.warnedNonDefaultReportFilename.has(projectKey)) { + this.warnedNonDefaultReportFilename.add(projectKey); + this.logger.showWarning( + `Ceedling project '${projectKey}': the CppUnit XML report filename resolves to '${reportFilename}', ` + + `not the default '${DEFAULT_CPPUNIT_REPORT_FILENAME}' - project.yml or a configured mixin sets ` + + `':report_tests_log_factory' ↳ ':cppunit' ↳ ':filename'. This extension will use the resolved name; ` + + `if that's unexpected, check ceedlingExplorer.mixins/projects[].mixins and project.yml.` + ); + } this.reportFilenames[projectKey] = reportFilename; } @@ -866,6 +889,7 @@ export class CeedlingEngine { os.tmpdir(), `ceedling-dumpconfig-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}.yml`, ); + this.logger.debug(`loadResolvedConfig(${projectKey}): resolving via 'ceedling dumpconfig' -> '${tmpPath}'`); const result = await this.execCeedling(['dumpconfig', quoteIfNeeded(tmpPath)], projectKey); if (result.error) { this.logger.error(`loadResolvedConfig(${projectKey}): 'ceedling dumpconfig' failed: ${util.format(result)}`); diff --git a/src/logger.ts b/src/logger.ts index 44e5924..c6764ef 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -45,6 +45,12 @@ export class Logger { vscode.window.showInformationMessage(message); } + public showWarning(...args: unknown[]) { + const message = this.format(...args); + this.warn(message); + vscode.window.showWarningMessage(message); + } + public showError(...args: unknown[]) { const message = this.format(...args); this.error(message); diff --git a/tests/manual/project.yml b/tests/manual/project.yml index df3a460..0974526 100644 --- a/tests/manual/project.yml +++ b/tests/manual/project.yml @@ -28,15 +28,10 @@ :plugins: :load_paths: [] :enabled: - - report_tests_log_factory - report_tests_pretty_stdout - -# Specify which reports you'd like from the log factory -:report_tests_log_factory: - :reports: - - json - - junit - - cppunit + # report_tests_log_factory (with its cppunit report) is deliberately NOT enabled here - the + # extension's own bundled mixin (assets/mixins/report-tests-log-factory.yml) always enables it. + # This project demonstrates that no manual plugin setup is required. :paths: :test: diff --git a/tests/unit/ceedlingEngine.test.ts b/tests/unit/ceedlingEngine.test.ts index ab63b38..f0db883 100644 --- a/tests/unit/ceedlingEngine.test.ts +++ b/tests/unit/ceedlingEngine.test.ts @@ -22,11 +22,20 @@ function fakeWorkspaceFolder(fsPath: string): vscode.WorkspaceFolder { } as unknown as vscode.WorkspaceFolder; } -function noopLogger(): Logger { +type RecordedCall = { level: string, message: string }; +type FakeLogger = Logger & { calls: RecordedCall[] }; + +// A no-op Logger by default; every call is also recorded in `.calls` for tests that need to +// assert on what was logged (the warning/debug-enrichment tests below). +function fakeLogger(): FakeLogger { + const calls: RecordedCall[] = []; + const record = (level: string) => (...args: unknown[]) => { calls.push({ level, message: String(args[0]) }); }; return { - trace() { }, debug() { }, info() { }, warn() { }, error() { }, - showInfo() { }, showError() { }, dispose() { }, - } as unknown as Logger; + trace: record('trace'), debug: record('debug'), info: record('info'), warn: record('warn'), error: record('error'), + showInfo: record('showInfo'), showError: record('showError'), showWarning: record('showWarning'), + dispose() { }, + calls, + } as unknown as FakeLogger; } function fakeConfig(settings: Record): vscode.WorkspaceConfiguration { @@ -62,10 +71,12 @@ function makeEngine(opts: { settings?: Record, isDevelopmentMode?: boolean, bundledMixinPath?: string, + logger?: FakeLogger, + workspacePath?: string, } = {}): CeedlingEngine { return new CeedlingEngine( - fakeWorkspaceFolder('/ws'), - noopLogger(), + fakeWorkspaceFolder(opts.workspacePath ?? '/ws'), + opts.logger ?? fakeLogger(), opts.isDevelopmentMode ?? false, { configProvider: () => fakeConfig(opts.settings ?? {}), @@ -272,3 +283,51 @@ suite('real `ceedling dumpconfig` output (captured against tests/manual, see tes assert.strictEqual((engine as any).reportFilenames['proj'], 'cppunit_tests_report.xml'); }); }); + +suite('setXmlReportPath: non-default report filename warning', () => { + test('the default filename never warns', () => { + const logger = fakeLogger(); + const engine = makeEngine({ logger }); + (engine as any).setXmlReportPath('proj', { ':report_tests_log_factory': { ':cppunit': { ':filename': 'cppunit_tests_report.xml' } } }); + assert.ok(!logger.calls.some((c) => c.level === 'showWarning')); + }); + + test('a non-default filename warns once, naming the project and the resolved filename', () => { + const logger = fakeLogger(); + const engine = makeEngine({ logger }); + (engine as any).setXmlReportPath('proj', { ':report_tests_log_factory': { ':cppunit': { ':filename': 'custom_report.xml' } } }); + const warnings = logger.calls.filter((c) => c.level === 'showWarning'); + assert.strictEqual(warnings.length, 1); + assert.match(warnings[0].message, /proj/); + assert.match(warnings[0].message, /custom_report\.xml/); + }); + + test('the same non-default filename does not warn again on a later discover() cycle', () => { + const logger = fakeLogger(); + const engine = makeEngine({ logger }); + const resolvedConfig = { ':report_tests_log_factory': { ':cppunit': { ':filename': 'custom_report.xml' } } }; + (engine as any).setXmlReportPath('proj', resolvedConfig); + (engine as any).setXmlReportPath('proj', resolvedConfig); + assert.strictEqual(logger.calls.filter((c) => c.level === 'showWarning').length, 1); + }); +}); + +suite('loadProjectPaths: mixins are logged for diagnosis', () => { + test('the debug log naming a resolved project includes its resolved mixins', () => { + const logger = fakeLogger(); + // loadProjectPaths() checks the workspace path against the real filesystem + // (fs.existsSync/lstatSync, not part of the injected seam) - point it at this test + // file's own real directory rather than the fake '/ws' the other tests use. + const engine = makeEngine({ + logger, + workspacePath: __dirname, + settings: { + projects: [{ path: '.', debugLaunchConfig: 'ceedling', mixins: ['gcc'] }], + }, + }); + (engine as any).loadProjectPaths(); + const line = logger.calls.find((c) => c.level === 'debug' && c.message.includes('loadProjectPaths')); + assert.ok(line, 'expected a loadProjectPaths debug log line'); + assert.match(line!.message, /mixins.*gcc/); + }); +}); From f22648a7b63b7c727e53272671f7198aaebfff3e Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Wed, 30 Sep 2026 13:04:40 -0400 Subject: [PATCH 08/10] Simplify getTestPrefix: :test_runner:test_prefix is the only real prefix setting :unity:test_prefix was never a real Ceedling option - a misunderstanding inherited from the original extension. The define-sniffing that chose between it and :test_runner:test_prefix modeled nothing about actual Ceedling/Unity behavior; it just picked between a real field and a fake one. Read :test_runner:test_prefix directly instead. Co-Authored-By: Claude Sonnet 5 --- src/ceedlingEngine.ts | 19 ++++--------------- tests/unit/ceedlingEngine.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 15 deletions(-) diff --git a/src/ceedlingEngine.ts b/src/ceedlingEngine.ts index b3f5b61..26eb119 100644 --- a/src/ceedlingEngine.ts +++ b/src/ceedlingEngine.ts @@ -690,23 +690,12 @@ export class CeedlingEngine { let testPrefix = 'test|spec|should'; if (ymlProjectData) { try { - const ymlProjectUnityDefines = ymlProjectData[':unity'][':defines']; - if (ymlProjectUnityDefines != undefined && ( - ymlProjectUnityDefines.includes('-UNITY_SKIP_DEFAULT_RUNNER') || - ymlProjectUnityDefines.includes('-RUN_TEST') - )) { - const ymlProjectTestRunnerTestPrefix = ymlProjectData[':test_runner'][':test_prefix']; - if (ymlProjectTestRunnerTestPrefix != undefined) { - testPrefix = ymlProjectTestRunnerTestPrefix; - } - } else { - const ymlProjectTestPrefix = ymlProjectData[':unity'][':test_prefix']; - if (ymlProjectTestPrefix != undefined) { - testPrefix = ymlProjectTestPrefix; - } + const ymlProjectTestPrefix = ymlProjectData[':test_runner'][':test_prefix']; + if (ymlProjectTestPrefix != undefined) { + testPrefix = ymlProjectTestPrefix; } } catch (e) { - this.logger.debug(`getTestPrefix: no ':unity'/':test_runner' data in project.yml, using default '${testPrefix}': ${e}`); + this.logger.debug(`getTestPrefix: no ':test_runner'/':test_prefix' in project.yml, using default '${testPrefix}': ${e}`); } } return testPrefix; diff --git a/tests/unit/ceedlingEngine.test.ts b/tests/unit/ceedlingEngine.test.ts index f0db883..104bc3a 100644 --- a/tests/unit/ceedlingEngine.test.ts +++ b/tests/unit/ceedlingEngine.test.ts @@ -284,6 +284,33 @@ suite('real `ceedling dumpconfig` output (captured against tests/manual, see tes }); }); +suite('getTestPrefix', () => { + test('defaults to test|spec|should when no project data is given', () => { + const engine = makeEngine({ logger: fakeLogger() }); + assert.strictEqual((engine as any).getTestPrefix(undefined), 'test|spec|should'); + }); + + test('uses :test_runner: :test_prefix: when present', () => { + const engine = makeEngine({ logger: fakeLogger() }); + const prefix = (engine as any).getTestPrefix({ ':test_runner': { ':test_prefix': 'my_test_prefix' } }); + assert.strictEqual(prefix, 'my_test_prefix'); + }); + + test(':unity: :test_prefix: is no longer recognized and is ignored', () => { + const engine = makeEngine({ logger: fakeLogger() }); + const prefix = (engine as any).getTestPrefix({ ':unity': { ':test_prefix': 'legacy_prefix' } }); + assert.strictEqual(prefix, 'test|spec|should'); + }); + + test('missing :test_runner: data logs a debug message and falls back to the default', () => { + const logger = fakeLogger(); + const engine = makeEngine({ logger }); + const prefix = (engine as any).getTestPrefix({}); + assert.strictEqual(prefix, 'test|spec|should'); + assert.ok(logger.calls.some((c) => c.level === 'debug' && /getTestPrefix/.test(c.message))); + }); +}); + suite('setXmlReportPath: non-default report filename warning', () => { test('the default filename never warns', () => { const logger = fakeLogger(); From 6f820cd55d824de5f40b628a94505ffaebb31188 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Wed, 30 Sep 2026 15:51:16 -0400 Subject: [PATCH 09/10] =?UTF-8?q?=F0=9F=93=9D=20Cleaned=20up?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removed improperly generated text and reformatted configuration values to documentation convention --- tests/fixtures/dumpconfig/README.md | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/tests/fixtures/dumpconfig/README.md b/tests/fixtures/dumpconfig/README.md index b617205..53d5237 100644 --- a/tests/fixtures/dumpconfig/README.md +++ b/tests/fixtures/dumpconfig/README.md @@ -5,20 +5,11 @@ Real `ceedling dumpconfig` output, captured against a copy of `project.yml`. Captured with Ceedling **1.1.8** (a released version within this extension's supported range). -An earlier capture used a Ceedling 1.2.0 development/prerelease checkout and turned -up custom `!ruby/object:FilenameExtension` tags under `:extension:` that `js-yaml`'s -safe loader refuses to parse. That's an unreleased regression in `dumpconfig` -itself — `configurator_builder.rb`'s `normalize_filename_extensions` wraps every -`:extension:` sub-key in a `FilenameExtension` value object for the build's own use, -and `dump_yaml` dumps that live object graph directly via `YAML.dump` instead of a -plain, portable copy. Confirmed absent on 1.1.8: `:extension:` values are plain -strings there, as expected. Will be fixed upstream before 1.2.0 ships. - - `manual-project-no-report-plugin.yml` — `ceedling dumpconfig` with no `--mixin`. - `:plugins: :enabled:` has no `report_tests_log_factory`. + `:plugins` ↳ `:enabled` has no `report_tests_log_factory`. - `manual-project-with-bundled-mixin.yml` — the same project, `--mixin - assets/mixins/report-tests-log-factory.yml` added. `:plugins: :enabled:` now - includes `report_tests_log_factory`, and `:report_tests_log_factory: :reports:` + assets/mixins/report-tests-log-factory.yml` added. `:plugins` ↳ `:enabled` now + includes `report_tests_log_factory`, and `:report_tests_log_factory` ↳ `:reports` includes `cppunit`. A full `ceedling test:all` with this same mixin was also run by hand (Ceedling 1.1.8) against the stripped project and did produce `build/artifacts/test/cppunit_tests_report.xml`, confirming the mixin is From 3217044eb181380ef43f4b7e8ac6cc4d61ca23f3 Mon Sep 17 00:00:00 2001 From: Michael Karlesky Date: Fri, 23 Oct 2026 09:33:07 -0400 Subject: [PATCH 10/10] =?UTF-8?q?=F0=9F=93=9D=20Doc=20updates=20for=20fixe?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 1 + docs/KnownIssues.md | 1 + docs/ReleaseNotes.md | 6 +++++- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d619f09..0c3acf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ This format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) ## ⚠️ Changed * Project configuration is now resolved with Ceedling’s own `ceedling dumpconfig` instead of the extension parsing and merging `project.yml` (and any mixin) itself. This reflects Ceedling’s actual merge — plugin defaults included — rather than a parallel one the extension maintained on its own. +* Dropped erroneous recognition of `:unity` ↳ `:test_prefix` from project configuration processing and the Unity compilation define sniffing that was used to select it. These were never real Ceedling options; the only supported way to change the test-case function prefix is `:test_runner` ↳ `:test_prefix`, which this extension continues to support unchanged. ## 🌟 Added diff --git a/docs/KnownIssues.md b/docs/KnownIssues.md index f78bc6e..d1b610d 100644 --- a/docs/KnownIssues.md +++ b/docs/KnownIssues.md @@ -8,3 +8,4 @@ Complemented by three other documents: [../CHANGELOG.md](../CHANGELOG.md), [Rele 1. Individual parametrized test-case debugging is not supported — Ceedling always builds and runs a whole test file’s executable, so debugging always targets the containing file, never a single `TEST_CASE`/`TEST_RANGE` case. 2. `compileForDebug` doesn't acquire the same mutex `runInternal` does around its Ceedling invocation and XML report read. A concurrent Run and Debug could theoretically race on the shared report file. +3. Test-case discovery in the Testing view is based on matching `:test_runner:test_prefix` against function names in source text. If a project uses Unity's `UNITY_SKIP_DEFAULT_RUNNER`/custom `RUN_TEST` to hand-write its own runner and invoke functions that don't follow the configured prefix convention, this extension cannot correlate those functions to test results — this is a fundamental limit of prefix-based source scanning, not something a configuration change can fix. diff --git a/docs/ReleaseNotes.md b/docs/ReleaseNotes.md index ca0f8d0..7520122 100644 --- a/docs/ReleaseNotes.md +++ b/docs/ReleaseNotes.md @@ -12,9 +12,13 @@ Project configuration is now resolved with Ceedling’s own `ceedling dumpconfig The `report_tests_log_factory` plugin’s `cppunit` report, which the Testing view depends on to read results, is now enabled automatically via a mixin bundled with the extension. Manually editing `project.yml` to enable it is no longer necessary. New `ceedlingExplorer.mixins` and `ceedlingExplorer.projects[].mixins` settings expose Ceedling’s own mixin mechanism directly, for anyone who wants to layer toolchain targets, CI overrides, or other configuration. See [Mixins](../README.md#mixins). +`:unity: :test_prefix:` was never a real Ceedling setting — a misunderstanding inherited from the original extension — and is no longer read. If you want to change the recognized prefix of test-case functions, set `:test_runner: :test_prefix:`, which Ceedling actually supports. + ## 🚨 Important Changes -None — existing `project.yml` files that already enable `report_tests_log_factory` themselves continue to work unchanged; the extension’s mixin is additive. +Existing `project.yml` files that already enable `report_tests_log_factory` themselves continue to work unchanged; the extension’s mixin is additive. + +If a `project.yml` set `:unity: :test_prefix:` *and* also passed `-UNITY_SKIP_DEFAULT_RUNNER` or `-RUN_TEST` as a Unity define expecting it to be ignored in favor of `:test_runner: :test_prefix:`, behavior is now simpler and more predictable: `:test_runner: :test_prefix:` is always what's used, with no define-sniffing. Projects using only `:test_runner: :test_prefix:` (the real, documented setting) are unaffected. ---