Conversation
…havior
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
… 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 <noreply@anthropic.com>
…erge
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…ename 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replaces the extension's custom in-process YAML/mixin merging with Ceedling's own
ceedling dumpconfig, and exposes Ceedling's mixin mechanism as first-class VS Code settings — a minor release, ahead of the planned MCP extraction work.getYmlProjectData/mergeYmlProjectData(direct file parse + hand-rolleddeepmerge) replaced withloadResolvedConfig, backed byceedling dumpconfig. Cached once perdiscover()cycle (one dumpconfig invocation per project, down from three independent file parses).assets/mixins/report-tests-log-factory.yml) always enablesreport_tests_log_factory'scppunitreport, always highest-priority. Manually editingproject.ymlis no longer required —tests/manual/project.ymlno longer enables it itself, demonstrating this.ceedlingExplorer.mixinsandceedlingExplorer.projects[].mixinsexpose Ceedling's own--mixinmechanism (toolchain targets, CI overrides, etc.), lowest to highest priority, with a documented absent-vs-empty-array fallback rule.src/ceedlingMixinArgs.tsbuilds and quotes the--project/--mixinflags for every invocation.CeedlingEnginewas previously untestable outside a real VS Code (transitivevscodeimports viaLogger/ProblemMatcher). Now takes injectedexec/readFile/configProvider/problemMatcher/bundledMixinPath;vscode/Logger/ProblemMatcherareimport typeonly. 78 unit tests now cover this file (was 0).showWarningnotification when the resolved CppUnit report filename isn't Ceedling's default, naming the project and filename.loadProjectPaths' debug log;loadResolvedConfiglogs itsdumpconfiginvocation.A real bug found and worked around during verification
Replaying real
ceedling dumpconfigoutput (not just synthetic YAML) surfaced custom!ruby/object:FilenameExtensiontags under:extension:thatjs-yaml's safe loader can't parse. Traced to an unreleased Ceedling 1.2.0-dev regression indump_yaml(dumps the live internal config object graph viaYAML.dumpinstead of a plain copy) — confirmed absent on released 1.1.8. Fixtures intests/fixtures/dumpconfig/were captured against 1.1.8; the maintainer is aware and will fix it upstream before 1.2.0 ships (seetests/fixtures/dumpconfig/README.mdfor full detail).Verification
npm run test:unit: 78 passing (was 53 before this branch).npx tsc --noEmit -p tsconfig.json: clean (one pre-existing, unrelated warning also present onmaster).npm run build+xvfb-run -a npm run test:integration: extension builds and activates cleanly in a real VS Code.tests/manual/project.yml(withreport_tests_log_factorygenuinely absent) run viaceedling test:all --mixin assets/mixins/report-tests-log-factory.ymlalone producesbuild/artifacts/test/cppunit_tests_report.xml.Commits
Structured as six reviewable steps: test infrastructure → the pure mixin-args helper → bundling + settings + wiring → the dumpconfig migration itself → docs → real-fixture verification, plus a final round addressing review feedback (manual fixture, logging, the new warning).
🤖 Generated with Claude Code