diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a34aa6..b63544e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ This document is the terse, itemized record of what changed in each release. Its * `:unity` ↳ `:test_prefix` is no longer read, nor the Unity compilation defines once sniffed to select it. Neither was ever a real Ceedling option. `:test_runner` ↳ `:test_prefix` remains the way to change the test function prefix. * A workspace folder without a Ceedling project no longer adds an empty _Ceedling_ entry to the _Testing_ view. With several workspace folders, each entry names its folder. * Discovery checks the Ceedling version in each project’s own directory, so a project’s `:project` ↳ `:which_ceedling` is respected. It no longer runs `ceedling summary`. +* Discovery is faster in workspaces with several project directories or many test files. Projects in different directories load at the same time, and test files are read in parallel. Reading files no longer blocks VS Code’s extension host. +* Ceedling commands in different project directories may now run at the same time, e.g. a run in one project while another rediscovers. Commands within one project directory still run one at a time. ## 🌟 Added @@ -27,6 +29,7 @@ This document is the terse, itemized record of what changed in each release. Its * Unity’s `TEST_MATRIX()` parametrized tests and exclusive `TEST_RANGE()` ranges now appear in the _Testing_ view as individual test cases. * New `ceedlingExplorer.ceedlingCommand` setting for running Ceedling through another command, e.g. `bundle exec ceedling`. * A one-time warning when `project.yml` or a configured mixin sets a non-default CppUnit report filename, naming the project and the filename. +* A warning in the _Ceedling Explorer_ log for a `TEST_RANGE()` with no `[start, end, step]` or `` range, or a `TEST_MATRIX()` with no `[ … ]` list. Ceedling can’t build such a test. ## 💪 Fixed diff --git a/docs/Development.md b/docs/Development.md index 07bbecb..d6b4566 100644 --- a/docs/Development.md +++ b/docs/Development.md @@ -62,6 +62,7 @@ The code has two layers. Most modules have no `vscode` dependency and are unit-t | `src/runPlan.ts` | Pure | A run request → the fewest Ceedling invocations | | `src/testModel.ts` | Pure | The test tree types and the `RunReporter` interface | | `src/singleFlight.ts` | Pure | Coalesces overlapping discovery requests into one rerun | +| `src/concurrency.ts` | Pure | Grouped concurrent mapping, a lock per key, and a limit shared by every caller | | `src/ceedlingOutputParsing.ts` | Pure | Ceedling stdout, report names, and pretty labels | | `src/ceedlingMixinArgs.ts` | Pure | The `--project`/`--mixin` flags for a project | | `src/problemMatching.ts` | Pure | Compiler-output patterns and their matches | @@ -74,10 +75,12 @@ Two Ceedling constraints shape everything else. Ceedling always compiles a whole 1. `projectsFromSettings()` turns `ceedlingExplorer.projects` and `ceedlingExplorer.mixins` into `CeedlingProject`s: key, absolute path, project file, launch configuration name, and mixins. Projects whose project file exists become the engine’s `candidates`. 2. For each candidate, `checkCeedlingVersion()` runs `ceedling version` in the project directory and requires 1.0.0 or later. `semver.coerce()` reads a build suffix such as `1.2.0-13a540f` as its release. -3. `resolveConfig()` runs `ceedling dumpconfig ` with the project’s flags and parses the YAML. `projectState()` reads the `ProjectState` from it: `buildRoot`, `reportFilename`, `testPrefix`, `testFilePrefix`, `executableExtension`, `useParamTests`, and `reportPluginEnabled`. +3. `resolveConfig()` runs `ceedling dumpconfig` with the project’s flags and parses the YAML. Ceedling 1.2.0 writes it to stdout (`--stdout`). Earlier versions write it to a uniquely named temporary file, which is then removed. `projectState()` reads the `ProjectState` from it: `buildRoot`, `reportFilename`, `testPrefix`, `testFilePrefix`, `executableExtension`, `useParamTests`, and `reportPluginEnabled`. 4. `listTestFiles()` runs `ceedling files:test` and parses its bullets with `parseFileListBullets()`. 5. `testFileNodes()` reads each file and calls `discoverTests(source, { testPrefix, useParamTests })`. A function’s id is `file::name`, and a case’s is `file::name(args)`, exactly the names in Ceedling’s report. +Discovery overlaps the work it safely can, since every Ceedling start costs a Ruby startup. Steps 2 through 4 run concurrently across project directories and one project at a time within a directory (`mapGrouped()`). Outcomes apply in settings order, so errors read the same however the work interleaves. Test sources are read concurrently, within one limit of 16 open reads shared by every discovery and reparse (`Limiter`), and parsed in listed order. + The engine replaces its `projects` map only after every project loads. A failed discovery throws `CeedlingLoadError` and leaves the last good map in place for runs. The controller then turns the tree into TestItems. It keeps a `PlanTarget` for each item, and item lookups per project, since ids are unique only within one project. ### Staying current @@ -96,10 +99,12 @@ The TestController itself exists only while discovery finds a project, so a fold A run turns the request into a plan, then reports each plan entry’s results. - **Planning.** `buildRunPlan(projects, include, exclude)` returns `RunPlanEntry`s of three kinds. A `project` entry runs `test:all`. A `file` entry runs `test:`. A `function` entry adds `--test-case=`. Excluded items narrow the plan to function granularity at most. The reporter drops their results. -- **Running.** `runTests()` marks every id started, then takes the engine’s mutex. `buildAndRun()` deletes stale reports in `artifacts/test` and `artifacts/gcov`, runs Ceedling, appends the transcript to the run’s output, updates diagnostics, and reads the newest report. +- **Running.** `runTests()` marks every id started, then takes its project directory’s lock. `buildAndRun()` deletes stale reports in `artifacts/test` and `artifacts/gcov`, runs Ceedling, appends the transcript to the run’s output, updates diagnostics, and reads the newest report. - **Reporting.** `testResults(report, leafIds)` yields `passed`, `skipped`, `failed`, and `unreported` results. An `unreported` id gets an error naming the likely cause: discovery and Ceedling disagree about its name. With no report at all, every id errors, and the most severe compiler diagnostic leads the message. - **Edge cases.** A `--test-case` run whose report also names another function, from Ceedling’s substring filter, reruns the whole file. A cancelled run kills only its own process tree, through `CeedlingRunner`, and marks its tests skipped. +**One Ceedling command per project directory at a time.** Every Ceedling command that loads a project, `dumpconfig` and `files:test` included, rewrites the vendored Unity and CMock sources under `build/vendor/` in place. A second command in the same directory can leave a build compiling a half-written header. So `dumpconfig`, `files:test`, runs, debug builds, and clean/clobber each hold their directory’s lock (`KeyedLock`, via `inDirectory()`). `ceedling version` loads no project and needs no lock. Different directories proceed independently, so builds in separate projects may overlap. + Diagnostics are keyed by run scope: the project key for a project run, `projectKey::fileId` for a file or function run. A project run replaces its own entry and drops its files’ entries. A file run replaces its own entry and drops the project entry’s diagnostics located in that file. ### Debugging diff --git a/package-lock.json b/package-lock.json index 5a35901..4c0797f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,7 +9,6 @@ "version": "2.0.1", "license": "MIT", "dependencies": { - "async-mutex": "^0.1.3", "js-yaml": "^3.13.1", "semver": "^7.5.4", "strip-ansi": "^6.0.0", @@ -2230,12 +2229,6 @@ "node": ">=8" } }, - "node_modules/async-mutex": { - "version": "0.1.4", - "resolved": "https://registry.npmjs.org/async-mutex/-/async-mutex-0.1.4.tgz", - "integrity": "sha512-zVWTmAnxxHaeB2B1te84oecI8zTDJ/8G49aVBblRX6be0oq6pAybNcUSxwfgVOmOjSCvN4aYZAqwtyNI8e1YGw==", - "license": "MIT" - }, "node_modules/asynckit": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/asynckit/-/asynckit-0.4.0.tgz", diff --git a/package.json b/package.json index 71dfefc..8acd67a 100644 --- a/package.json +++ b/package.json @@ -59,7 +59,6 @@ "lint": "eslint ." }, "dependencies": { - "async-mutex": "^0.1.3", "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 ba426df..ca3c674 100644 --- a/src/ceedlingEngine.ts +++ b/src/ceedlingEngine.ts @@ -10,7 +10,6 @@ // It has no runtime `vscode` dependency. Workspace configuration, the problem matcher, the // runner, and the filesystem come in through `deps`, so the unit tests drive it with fakes. -import { Mutex } from 'async-mutex'; import fs from 'fs'; import yaml from 'js-yaml'; import os from 'os'; @@ -19,6 +18,7 @@ import semver from 'semver'; import util from 'util'; import type vscode from 'vscode'; import { buildProjectArgs } from './ceedlingMixinArgs'; +import { KeyedLock, Limiter, mapGrouped } from './concurrency'; import { buildFileLabelRegex, buildTestLabelRegex, @@ -52,25 +52,37 @@ export { CeedlingLoadError } from './ceedlingProjectConfig'; const MINIMUM_CEEDLING_VERSION = '1.0.0'; // Added to test runs and debug builds in development mode only, for the extension's own logs. const DEBUG_VERBOSITY = ['--verbosity', 'debug']; +// Enough to overlap file reads, few enough to stay clear of file descriptor limits. +const MAX_CONCURRENT_READS = 16; const UNREPORTED_MESSAGE = "Ceedling reported no result for this test. Its name as found in the source may not match the name Ceedling's test runner gave it."; -// The filesystem operations the engine needs. Tests substitute an in-memory implementation. +// The filesystem operations the engine needs, all asynchronous: synchronous I/O would block the +// extension host thread every extension shares. Tests substitute an in-memory implementation. export interface EngineFileSystem { - exists(filePath: string): boolean; - isDirectory(filePath: string): boolean; + exists(filePath: string): Promise; + isDirectory(filePath: string): Promise; readFile(filePath: string): Promise; unlink(filePath: string): Promise; // undefined when the file doesn't exist. - mtimeMs(filePath: string): number | undefined; + mtimeMs(filePath: string): Promise; +} + +// A missing or unreadable path is undefined rather than an error. +async function statOrUndefined(filePath: string): Promise { + try { + return await fs.promises.stat(filePath); + } catch { + return undefined; + } } export const nodeFileSystem: EngineFileSystem = { - exists: (filePath) => fs.existsSync(filePath), - isDirectory: (filePath) => fs.statSync(filePath, { throwIfNoEntry: false })?.isDirectory() ?? false, + exists: async (filePath) => (await statOrUndefined(filePath)) !== undefined, + isDirectory: async (filePath) => (await statOrUndefined(filePath))?.isDirectory() ?? false, readFile: (filePath) => fs.promises.readFile(filePath, 'utf8'), unlink: (filePath) => fs.promises.unlink(filePath), - mtimeMs: (filePath) => fs.statSync(filePath, { throwIfNoEntry: false })?.mtimeMs, + mtimeMs: async (filePath) => (await statOrUndefined(filePath))?.mtimeMs, }; export type EngineDeps = { @@ -86,6 +98,11 @@ export type EngineDeps = { type LoadedProject = CeedlingProject & { capabilities: CeedlingCapabilities, state: ProjectState, files: CeedlingFileNode[] }; +type LoadOutcome = + | { kind: 'loaded', project: LoadedProject } + | { kind: 'error', message: string } + | { kind: 'unusable', error: unknown }; + // What a run compiled. A file run's diagnostics replace only that file's, a project run's all. type RunScope = { kind: 'project' } | { kind: 'file', file: CeedlingFileNode }; @@ -110,8 +127,14 @@ export class CeedlingEngine { // Path rules follow `platform` rather than the host's, as ceedlingProjectConfig's do. private readonly paths: typeof path.posix; private readonly tmpdir: string; - // Serializes every Ceedling invocation that builds or deletes in a project's build directory. - private readonly mutex = new Mutex(); + // One Ceedling command at a time per project directory. Every command that loads a project + // rewrites the Unity and CMock copies in its build directory, in place, so a command overlapping + // a build in the same directory can corrupt it. Different directories proceed independently. + // `ceedling version` loads no project and needs no lock. + private readonly directoryLocks = new KeyedLock(); + // Every source read in the engine shares one limit, so overlapping discoveries and a burst of + // watcher-driven reparses (e.g. a branch checkout) can't exhaust file descriptors together. + private readonly reads = new Limiter(MAX_CONCURRENT_READS); // The last successful discovery. A failed discovery leaves it in place, so runs already under // way, and runs of the tree still on screen, keep their project arguments and directory. @@ -160,10 +183,10 @@ export class CeedlingEngine { // default project files, any alternate project file, and each mixin given as a file path. // Watching these starts discovery when one appears, changes, or goes away. A mixin given by // name resolves through Ceedling's load paths, which aren't known here, so it isn't included. - configurationFiles(): string[] { + async configurationFiles(): Promise { let projects: CeedlingProject[]; try { - projects = this.projectSettings(); + projects = await this.projectSettings(); } catch { return []; } @@ -213,62 +236,49 @@ export class CeedlingEngine { // Discovers every configured project and its tests. Throws CeedlingLoadError when Ceedling or // a project's configuration is unusable. A folder without a project file is zero projects. async discover(): Promise { - const settings = this.projectSettings(); - this.candidates = settings.filter((project) => { - const projectFile = this.paths.resolve(project.absPath, project.ymlFileName); - const exists = this.fs.exists(projectFile); + const settings = await this.projectSettings(); + const projectFiles = settings.map((project) => this.paths.resolve(project.absPath, project.ymlFileName)); + const present = await Promise.all(projectFiles.map((projectFile) => this.fs.exists(projectFile))); + this.candidates = settings.filter((project, i) => { this.logger.debug( - `discover: project '${project.key}' -> ${projectFile}${exists ? '' : ' (absent, skipped)'}, mixins=${util.format(project.mixins)}` + `discover: project '${project.key}' -> ${projectFiles[i]}${present[i] ? '' : ' (absent, skipped)'}, mixins=${util.format(project.mixins)}` ); - return exists; + return present[i]; }); if (this.candidates.length === 0) { this.projects = new Map(); return []; } + // Projects load concurrently across directories, one at a time within a directory. Outcomes + // apply in settings order, so errors read the same however the loads interleave. + const outcomes = await mapGrouped(this.candidates, (project) => project.absPath, (project) => this.loadProject(project)); const loaded = new Map(); const errors: string[] = []; - for (const project of this.candidates) { - const capabilities = capabilitiesOf(await this.checkCeedlingVersion(project)); - if (project.ambiguousProjectFile && capabilities.ceedlingYml) { - errors.push( - `Both project.yml and ceedling.yml exist in ${project.absPath}, so Ceedling can't tell which is project '${project.key}'. ` + - `Remove or rename one, or name the one to use in ceedlingExplorer.projects[].path.` - ); - continue; + for (const outcome of outcomes) { + if (outcome.kind === 'unusable') { + throw outcome.error; } - const resolved = await this.resolveConfig(project, capabilities); - if (resolved === undefined) { - errors.push( - `Failed to resolve Ceedling's configuration for project '${project.key}' with 'ceedling dumpconfig'. ` + - `Check that Ceedling runs in the configured shell (ceedlingExplorer.shellPath), and that ` + - `ceedlingExplorer.projects, ceedlingExplorer.mixins, and any projects[].mixins name valid files and mixins.` - ); + if (outcome.kind === 'error') { + errors.push(outcome.message); continue; } - const state = projectState(resolved, this.platform); - this.logger.debug(`discover: project '${project.key}' state=${util.format(state)}`); - if (!state.reportPluginEnabled) { - // The bundled mixin always enables the plugin, so the mixin itself didn't apply. - errors.push( - `The required Ceedling plugin 'report_tests_log_factory' did not take effect for project '${project.key}', ` + - `even though this extension's own mixin always enables it. Check that ceedlingExplorer.mixins and ` + - `projects[].mixins don't reference an invalid mixin.` - ); - continue; - } - this.warnIfNonDefaultReportFilename(project.key, state.reportFilename); - loaded.set(project.key, { ...project, capabilities, state, files: [] }); + this.warnIfNonDefaultReportFilename(outcome.project.key, outcome.project.state.reportFilename); + loaded.set(outcome.project.key, outcome.project); } if (errors.length > 0) { throw new CeedlingLoadError(errors.join('\n')); } + // Listing runs Ceedling in each project directory, under its lock. Reading and parsing + // sources needs no lock. + const projects = [...loaded.values()]; + const listings = await mapGrouped(projects, (project) => project.absPath, + (project) => this.inDirectory(project, () => this.listTestFiles(project))); const labels = this.prettyLabels(); - for (const project of loaded.values()) { - project.files = await this.testFileNodes(project, await this.listTestFiles(project), labels); - } + await Promise.all(projects.map(async (project, i) => { + project.files = await this.testFileNodes(project, listings[i] ?? [], labels); + })); this.projects = loaded; const diagnosticsIds = new Set([...loaded.values()].flatMap((p) => [p.key, ...p.files.map((f) => fileDiagnosticsId(p.key, f.id))])); this.problemMatcher.retainIds((id) => diagnosticsIds.has(id)); @@ -317,7 +327,7 @@ export class CeedlingEngine { for (const id of ids) { reporter.started(id); } - return this.exclusive(async () => { + return this.inDirectory(project, async () => { const outcome = await this.buildAndRun(project, { kind: 'file', file }, this.testCommandArgs(project, file.id), reporter, token); if (outcome.cancelled) { leaves.forEach((id) => reporter.skipped(id)); @@ -328,7 +338,7 @@ export class CeedlingEngine { return undefined; } this.reportResults(outcome.report, leaves, outcome.transcript, reporter); - return this.executableSubpath(project, file); + return await this.executableSubpath(project, file); }); } @@ -345,7 +355,7 @@ export class CeedlingEngine { this.runner.cancelAll(); } - private projectSettings(): CeedlingProject[] { + private projectSettings(): Promise { const config = this.getConfiguration(); return projectsFromSettings( { projects: config.get('projects', []), mixins: config.get('mixins', []) }, @@ -366,13 +376,47 @@ export class CeedlingEngine { return project; } - private async exclusive(work: () => Promise): Promise { - const release = await this.mutex.acquire(); + private inDirectory(project: CeedlingProject, work: () => Promise): Promise { + return this.directoryLocks.run(project.absPath, work); + } + + // Checks one project's Ceedling and resolves its configuration. An unusable Ceedling stops all + // discovery. A configuration problem is reported alongside any others. + private async loadProject(project: CeedlingProject): Promise { + let capabilities: CeedlingCapabilities; try { - return await work(); - } finally { - release(); + capabilities = capabilitiesOf(await this.checkCeedlingVersion(project)); + } catch (error) { + return { kind: 'unusable', error }; + } + if (project.ambiguousProjectFile && capabilities.ceedlingYml) { + return { + kind: 'error', + message: `Both project.yml and ceedling.yml exist in ${project.absPath}, so Ceedling can't tell which is project '${project.key}'. ` + + `Remove or rename one, or name the one to use in ceedlingExplorer.projects[].path.`, + }; + } + const resolved = await this.inDirectory(project, () => this.resolveConfig(project, capabilities)); + if (resolved === undefined) { + return { + kind: 'error', + message: `Failed to resolve Ceedling's configuration for project '${project.key}' with 'ceedling dumpconfig'. ` + + `Check that Ceedling runs in the configured shell (ceedlingExplorer.shellPath), and that ` + + `ceedlingExplorer.projects, ceedlingExplorer.mixins, and any projects[].mixins name valid files and mixins.`, + }; + } + const state = projectState(resolved, this.platform); + this.logger.debug(`loadProject: project '${project.key}' state=${util.format(state)}`); + if (!state.reportPluginEnabled) { + // The bundled mixin always enables the plugin, so the mixin itself didn't apply. + return { + kind: 'error', + message: `The required Ceedling plugin 'report_tests_log_factory' did not take effect for project '${project.key}', ` + + `even though this extension's own mixin always enables it. Check that ceedlingExplorer.mixins and ` + + `projects[].mixins don't reference an invalid mixin.`, + }; } + return { kind: 'loaded', project: { ...project, capabilities, state, files: [] } }; } // Runs in the project's own directory, so `:project: :which_ceedling:` selects the Ceedling @@ -423,7 +467,7 @@ export class CeedlingEngine { return undefined; } try { - return yaml.safeLoad(await this.fs.readFile(outputPath)); + return yaml.safeLoad(await this.reads.run(() => this.fs.readFile(outputPath))); } catch (e) { this.logger.error(`resolveConfig(${project.key}): could not read dumpconfig output '${outputPath}': ${util.format(e)}`); return undefined; @@ -445,15 +489,14 @@ export class CeedlingEngine { ); } - private listTestFiles(project: CeedlingProject): Promise { - return this.exclusive(async () => { - const result = await this.ceedling(['files:test'], { project }); - if (result.error) { - this.logger.error(`listTestFiles(${project.key}): 'ceedling files:test' failed: ${util.format(result)}`); - return []; - } - return parseFileListBullets(result.stdout); - }); + // Callers hold the build lock. + private async listTestFiles(project: CeedlingProject): Promise { + const result = await this.ceedling(['files:test'], { project }); + if (result.error) { + this.logger.error(`listTestFiles(${project.key}): 'ceedling files:test' failed: ${util.format(result)}`); + return []; + } + return parseFileListBullets(result.stdout); } // `source`, when given, stands in for the contents of a single file. @@ -464,12 +507,24 @@ export class CeedlingEngine { // `||`, not `??`: an empty capture, such as a file named only by its prefix, keeps the full text. // eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing -- see above const label = (regex: RegExp | undefined, text: string) => regex?.exec(text)?.[1] || text; - const nodes: CeedlingFileNode[] = []; - for (const file of files) { + // Sources are read concurrently, within the engine's shared read limit, then parsed in + // listed order. + const sources = await Promise.all(files.map(async (file) => { const absPath = this.paths.resolve(project.absPath, file); + try { + return { file, absPath, text: source ?? await this.reads.run(() => this.fs.readFile(absPath)) }; + } catch (error) { + return { file, absPath, error }; + } + })); + const nodes: CeedlingFileNode[] = []; + for (const { file, absPath, text, error } of sources) { let discovered: ReturnType; try { - discovered = discoverTests(source ?? await this.fs.readFile(absPath), options); + if (text === undefined) { + throw error; + } + discovered = discoverTests(text, options); } catch (e) { // An unreadable file, or a :test_runner: :test_prefix: that isn't a valid regex. this.logger.warn(`testFileNodes: skipping '${absPath}' (test prefix '${options.testPrefix}'): ${util.format(e)}`); @@ -513,7 +568,7 @@ export class CeedlingEngine { for (const id of ids) { reporter.started(id); } - return this.exclusive(async () => { + return this.inDirectory(project, async () => { const outcome = await this.buildAndRun(project, scope, args, reporter, token); if (outcome.cancelled) { leaves.forEach((id) => reporter.skipped(id)); @@ -609,15 +664,15 @@ export class CeedlingEngine { // `name/name`. Ceedling 1.2.0 nests that under the test file's directory relative to its // test path, e.g. `unit/name/name` for `test/unit/name.c`. That test path isn't known // here, so each trailing part of the file's directory is tried, and the newest build wins. - private executableSubpath(project: LoadedProject, file: CeedlingFileNode): string { + private async executableSubpath(project: LoadedProject, file: CeedlingFileNode): Promise { const segments = file.id.split(/[\\/]/); const name = (segments.pop() ?? file.id).replace(/\.[^.]*$/, ''); const executable = `${name}/${name}${project.state.executableExtension}`; const outputDirectory = this.paths.resolve(project.absPath, project.state.buildRoot, 'test', 'out'); - const [newest] = segments - .map((_, i) => [...segments.slice(i), executable].join('/')) - .concat(executable) - .map((subpath) => ({ subpath, mtime: this.fs.mtimeMs(this.paths.resolve(outputDirectory, subpath)) })) + const candidates = segments.map((_, i) => [...segments.slice(i), executable].join('/')).concat(executable); + const mtimes = await Promise.all(candidates.map((subpath) => this.fs.mtimeMs(this.paths.resolve(outputDirectory, subpath)))); + const [newest] = candidates + .map((subpath, i) => ({ subpath, mtime: mtimes[i] })) .filter((candidate): candidate is { subpath: string, mtime: number } => candidate.mtime !== undefined) .sort((a, b) => b.mtime - a.mtime); if (!newest) { @@ -632,15 +687,21 @@ export class CeedlingEngine { } private async deleteReports(project: LoadedProject): Promise { - for (const reportPath of this.reportPaths(project).filter((p) => this.fs.exists(p))) { - await this.fs.unlink(reportPath).catch((e) => this.logger.warn(`Failed to delete stale report '${reportPath}': ${e}`)); - } + await Promise.all(this.reportPaths(project).map(async (reportPath) => { + if (await this.fs.exists(reportPath)) { + await this.fs.unlink(reportPath).catch((e: unknown) => { + this.logger.warn(`Failed to delete stale report '${reportPath}': ${util.format(e)}`); + }); + } + })); } // The newer report, when a run such as `test:x gcov:x` wrote both. private async readReport(project: LoadedProject): Promise { - const [newest] = this.reportPaths(project) - .map((reportPath) => ({ reportPath, mtime: this.fs.mtimeMs(reportPath) })) + const reportPaths = this.reportPaths(project); + const mtimes = await Promise.all(reportPaths.map((reportPath) => this.fs.mtimeMs(reportPath))); + const [newest] = reportPaths + .map((reportPath, i) => ({ reportPath, mtime: mtimes[i] })) .filter((r): r is { reportPath: string, mtime: number } => r.mtime !== undefined) .sort((a, b) => b.mtime - a.mtime); if (!newest) { @@ -648,7 +709,7 @@ export class CeedlingEngine { return undefined; } try { - return await parseCppUnitReport(await this.fs.readFile(newest.reportPath)); + return await parseCppUnitReport(await this.reads.run(() => this.fs.readFile(newest.reportPath))); } catch (e) { this.logger.warn(`readReport(${project.key}): could not read '${newest.reportPath}': ${util.format(e)}`); return undefined; @@ -656,15 +717,9 @@ export class CeedlingEngine { } private async runInEveryProject(args: string[], token?: Cancellation): Promise { - return this.exclusive(async () => { - let succeeded = true; - for (const project of this.candidates) { - // eslint-disable-next-line no-await-in-loop -- one project at a time, under the build mutex - const result = await this.ceedling(args, { project, cancellation: token }); - succeeded = succeeded && !result.error; - } - return succeeded; - }); + const results = await mapGrouped(this.candidates, (project) => project.absPath, + (project) => this.inDirectory(project, () => this.ceedling(args, { project, cancellation: token }))); + return results.every((result) => !result.error); } // `${TEST_ID}` in each configured argument becomes the test file: its project-relative path diff --git a/src/ceedlingProcess.ts b/src/ceedlingProcess.ts index 9a0bb0c..2662754 100644 --- a/src/ceedlingProcess.ts +++ b/src/ceedlingProcess.ts @@ -125,12 +125,16 @@ export class CeedlingRunner { } } +// The one tree-kill call made, without its overloads, so a test double is easy to write. +type TreeKill = (pid: number, signal: string, callback: (error?: Error) => void) => void; + // Kills a process and every process beneath it. Linux reads the tree from /proc. tree-kill would // spawn `ps` there, which minimal containers lack, and its unhandled spawn error would crash the // extension host. Elsewhere tree-kill uses taskkill (Windows) or pgrep (macOS), both always -// present. A failure falls back to killing the root process alone. -export function killProcessTree(pid: number): void { - if (process.platform === 'linux') { +// present. A failure falls back to killing the root process alone. Tests substitute `platform` +// and `treeKill` to reach that fallback. +export function killProcessTree(pid: number, deps: { platform?: NodeJS.Platform, treeKill?: TreeKill } = {}): void { + if ((deps.platform ?? process.platform) === 'linux') { for (const target of descendantsOf(pid, linuxProcessParents())) { try { process.kill(target, 'SIGTERM'); @@ -140,7 +144,7 @@ export function killProcessTree(pid: number): void { } return; } - treeKill(pid, 'SIGTERM', (error) => { + (deps.treeKill ?? treeKill)(pid, 'SIGTERM', (error) => { if (error) { try { process.kill(pid, 'SIGTERM'); diff --git a/src/ceedlingProjectConfig.ts b/src/ceedlingProjectConfig.ts index ce200d8..d45a072 100644 --- a/src/ceedlingProjectConfig.ts +++ b/src/ceedlingProjectConfig.ts @@ -75,14 +75,15 @@ export function capabilitiesOf(version: string): CeedlingCapabilities { // Projects in settings order. With none configured, one default project sits at the workspace // root. A later entry with a duplicate key replaces an earlier one. `exists` decides which -// default project file a directory uses. -export function projectsFromSettings( +// default project file a directory uses. Every project's filesystem checks run concurrently, and +// their results apply in settings order, so a missing directory is reported deterministically. +export async function projectsFromSettings( settings: { projects: readonly ProjectSetting[], mixins: readonly string[] }, workspacePath: string, platform: NodeJS.Platform, - isDirectory: (absPath: string) => boolean, - exists: (absPath: string) => boolean, -): CeedlingProject[] { + isDirectory: (absPath: string) => Promise, + exists: (absPath: string) => Promise, +): Promise { // Path rules follow the given platform rather than the host's, so Windows paths resolve as // Windows paths wherever this runs. Gcov's XML report generation fails on Windows unless the // drive letter is uppercase. @@ -90,12 +91,15 @@ export function projectsFromSettings( const root = platform === 'win32' ? workspacePath.charAt(0).toUpperCase() + workspacePath.slice(1) : workspacePath; if (settings.projects.length === 0) { const absPath = paths.resolve(root); - return [{ key: 'default', absPath, ...projectFiles(absPath, undefined, paths, exists), debugLaunchConfig: 'ceedling', mixins: [...settings.mixins] }]; + return [{ key: 'default', absPath, ...(await projectFiles(absPath, undefined, paths, exists)), debugLaunchConfig: 'ceedling', mixins: [...settings.mixins] }]; } + const checked = await Promise.all(settings.projects.map(async (setting) => { + const project = await projectFromSetting(setting, settings.mixins, paths.resolve(root), paths, exists); + return { project, directory: await isDirectory(project.absPath) }; + })); const projects = new Map(); - for (const setting of settings.projects) { - const project = projectFromSetting(setting, settings.mixins, paths.resolve(root), paths, exists); - if (!isDirectory(project.absPath)) { + for (const { project, directory } of checked) { + if (!directory) { throw new CeedlingLoadError(`The project path ${project.absPath} does not exist or is not a directory.`); } projects.set(project.key, project); @@ -105,9 +109,9 @@ export function projectsFromSettings( // A path ending in .yml names a project file. Any other path names a directory holding a default // project file. The key is the configured name, else an alternate file's base name, else the path. -function projectFromSetting( - setting: ProjectSetting, globalMixins: readonly string[], root: string, paths: typeof path.posix, exists: (absPath: string) => boolean, -): CeedlingProject { +async function projectFromSetting( + setting: ProjectSetting, globalMixins: readonly string[], root: string, paths: typeof path.posix, exists: (absPath: string) => Promise, +): Promise { let directory = setting.path; let configuredFile: string | undefined; let key = setting.path; @@ -123,7 +127,7 @@ function projectFromSetting( return { key: setting.name ?? key, absPath, - ...projectFiles(absPath, configuredFile, paths, exists), + ...(await projectFiles(absPath, configuredFile, paths, exists)), debugLaunchConfig: setting.debugLaunchConfig, mixins: [...(setting.mixins ?? globalMixins)], }; @@ -131,11 +135,13 @@ function projectFromSetting( // The base project file is project.yml unless only ceedling.yml exists. A configured default // project file is itself the base. Any other configured file applies over the base as a mixin. -function projectFiles( - absPath: string, configuredFile: string | undefined, paths: typeof path.posix, exists: (absPath: string) => boolean, -): Pick { - const hasDefault = exists(paths.join(absPath, DEFAULT_PROJECT_FILE)); - const hasAlternate = exists(paths.join(absPath, ALTERNATE_DEFAULT_PROJECT_FILE)); +async function projectFiles( + absPath: string, configuredFile: string | undefined, paths: typeof path.posix, exists: (absPath: string) => Promise, +): Promise> { + const [hasDefault, hasAlternate] = await Promise.all([ + exists(paths.join(absPath, DEFAULT_PROJECT_FILE)), + exists(paths.join(absPath, ALTERNATE_DEFAULT_PROJECT_FILE)), + ]); const isDefaultName = configuredFile === DEFAULT_PROJECT_FILE || configuredFile === ALTERNATE_DEFAULT_PROJECT_FILE; const projectFileName = isDefaultName ? configuredFile : (hasAlternate && !hasDefault ? ALTERNATE_DEFAULT_PROJECT_FILE : DEFAULT_PROJECT_FILE); return { diff --git a/src/concurrency.ts b/src/concurrency.ts new file mode 100644 index 0000000..4fdf6a1 --- /dev/null +++ b/src/concurrency.ts @@ -0,0 +1,97 @@ +// Concurrency control for asynchronous work: grouped mapping that keeps input order, a lock per +// key, and a limit shared by every caller. + +type Entry = { item: T, index: number }; +type Failure = { reason: unknown }; + +// Runs `work` for every item, concurrently across groups and one item at a time within a group. +// Results keep the items' order. At the first failure no new work starts, work already in flight +// finishes, and then the first failure rejects, so nothing keeps running after the caller moves +// on. Two Ceedling processes in one project directory, for example, could collide on its build +// files, while different directories can't. +export async function mapGrouped(items: readonly T[], groupOf: (item: T) => string, work: (item: T) => Promise): Promise { + const groups = new Map>>(); + items.forEach((item, index) => { + const key = groupOf(item); + groups.set(key, [...(groups.get(key) ?? []), { item, index }]); + }); + const results = new Array(items.length); + const state: { failure?: Failure } = {}; + await Promise.all([...groups.values()].map(async (members) => { + for (const entry of members) { + if (state.failure) { + return; + } + // eslint-disable-next-line no-await-in-loop -- one item at a time within a group + await run(entry, work, results, state); + } + })); + if (state.failure) { + throw state.failure.reason; + } + return results; +} + +// One item's work, its result stored at its index, its failure recorded once, first wins. +async function run(entry: Entry, work: (item: T) => Promise, results: R[], state: { failure?: Failure }): Promise { + try { + results[entry.index] = await work(entry.item); + } catch (reason) { + state.failure ??= { reason }; + } +} + +// Runs work one at a time per key, in arrival order, while different keys proceed independently. +// Each run waits for the work queued before it under its key, whether that succeeded or failed. +export class KeyedLock { + // The settled-when-free promise for each busy key: the end of its queue. + private readonly tails = new Map>(); + + // Keys with work queued or running. Idle keys hold no state. + get size(): number { + return this.tails.size; + } + + async run(key: string, work: () => Promise): Promise { + const previous = this.tails.get(key) ?? Promise.resolve(); + let release!: () => void; + const tail = previous.then(() => new Promise((resolve) => { release = resolve; })); + this.tails.set(key, tail); + await previous; + try { + return await work(); + } finally { + release(); + if (this.tails.get(key) === tail) { + this.tails.delete(key); + } + } + } +} + +// Runs at most `limit` pieces of work at once, across every caller, starting queued work in +// arrival order. A finishing piece hands its slot straight to the next one waiting. +export class Limiter { + private active = 0; + private readonly waiting: Array<() => void> = []; + + constructor(private readonly limit: number) { } + + async run(work: () => Promise): Promise { + if (this.active < this.limit) { + this.active++; + } else { + await new Promise((resolve) => this.waiting.push(resolve)); + } + try { + return await work(); + } finally { + const next = this.waiting.shift(); + if (next) { + next(); + } else { + this.active--; + } + } + } +} diff --git a/src/testController.ts b/src/testController.ts index 8c6936b..2807e65 100644 --- a/src/testController.ts +++ b/src/testController.ts @@ -165,14 +165,14 @@ export class CeedlingTestController { this.projects = []; } this.rebuildTree(); - this.updateWatchers(); + await this.updateWatchers(); } // Configuration files rediscover everything. A C source change in a project directory // reparses that test file alone. A test file appearing or disappearing rediscovers, since only // `ceedling files:test` knows the full list. - private updateWatchers(): void { - const configurationFiles = new Set(this.engine.configurationFiles()); + private async updateWatchers(): Promise { + const configurationFiles = new Set(await this.engine.configurationFiles()); const projectDirectories = new Set(this.projects.map((p) => this.engine.getProjectAbsPath(p.projectKey))); this.syncWatchers(this.configurationWatchers, configurationFiles, (file) => new vscode.RelativePattern(vscode.Uri.file(path.dirname(file)), path.basename(file)), diff --git a/src/unityTestDiscovery.ts b/src/unityTestDiscovery.ts index e0a7214..48f058d 100644 --- a/src/unityTestDiscovery.ts +++ b/src/unityTestDiscovery.ts @@ -126,6 +126,10 @@ function expandRangeMarker(name: string, body: string, warnings: string[]): stri } ranges.push(values); } + if (ranges.length === 0) { + warnings.push(`TEST_RANGE for ${name} has no [start, end, step] or ranges; Ceedling cannot build this test.`); + return []; + } return cartesianProduct(ranges); } @@ -148,10 +152,8 @@ function expandMatrixMarker(name: string, body: string, warnings: string[]): str } // Every combination of one value per list, first list outermost, each joined as Unity joins it. +// Callers pass at least one list. Unity's generator fails on a marker with none. function cartesianProduct(lists: string[][]): string[] { - if (lists.length === 0) { - return []; - } return lists .reduce((combos, list) => combos.flatMap((combo) => list.map((value) => [...combo, value])), [[]]) .map((combo) => combo.join(', ')); diff --git a/tests/unit/ceedlingEngine.test.ts b/tests/unit/ceedlingEngine.test.ts index 8906f27..82eefdc 100644 --- a/tests/unit/ceedlingEngine.test.ts +++ b/tests/unit/ceedlingEngine.test.ts @@ -1,9 +1,10 @@ import * as assert from 'assert'; import type * as child_process from 'child_process'; import * as fs from 'fs'; +import * as os from 'os'; import * as path from 'path'; import type vscode from 'vscode'; -import { CeedlingEngine, type EngineFileSystem } from '../../src/ceedlingEngine'; +import { CeedlingEngine, type EngineFileSystem, nodeFileSystem } from '../../src/ceedlingEngine'; import { type Cancellation, CeedlingRunner } from '../../src/ceedlingProcess'; import { CeedlingLoadError } from '../../src/ceedlingProjectConfig'; import type { Logger } from '../../src/logger'; @@ -27,12 +28,19 @@ class FakeCeedling { files = new Map(); mtimes = new Map(); directories = new Set([WS]); - commands: Array<{ command: string, cwd: string }> = []; + commands: Array<{ command: string, cwd: string, shell?: string }> = []; handlers: Handler[] = []; version = '1.2.0'; resolvedConfig = `:plugins:\n :enabled:\n - report_tests_log_factory\n:unity:\n :use_param_tests: true\n`; testFiles = ['test/test_calc.c']; killed: number[] = []; + reads: string[] = []; + readsInFlight = 0; + maxReadsInFlight = 0; + // Paths that refuse to be deleted, as a file held open on Windows does. + undeletable = new Set(); + // While set, every file read waits for it, so a test can observe reads in flight. + readGate: Promise | undefined; private clock = 1; constructor() { @@ -48,15 +56,23 @@ class FakeCeedling { get fileSystem(): EngineFileSystem { return { - exists: (p) => this.files.has(p), - isDirectory: (p) => this.directories.has(p), + exists: async (p) => this.files.has(p), + isDirectory: async (p) => this.directories.has(p), readFile: async (p) => { + this.reads.push(p); + this.readsInFlight++; + this.maxReadsInFlight = Math.max(this.maxReadsInFlight, this.readsInFlight); + await this.readGate; + this.readsInFlight--; const content = this.files.get(p); if (content === undefined) throw Object.assign(new Error(`ENOENT: ${p}`), { code: 'ENOENT' }); return content; }, - unlink: async (p) => { this.files.delete(p); }, - mtimeMs: (p) => this.mtimes.get(p), + unlink: async (p) => { + if (this.undeletable.has(p)) throw Object.assign(new Error(`EPERM: ${p}`), { code: 'EPERM' }); + this.files.delete(p); + }, + mtimeMs: async (p) => this.mtimes.get(p), }; } @@ -64,7 +80,7 @@ class FakeCeedling { let pid = 1000; const exec = ((command: string, options: child_process.ExecOptions, callback: any) => { const cwd = String(options.cwd); - this.commands.push({ command, cwd }); + this.commands.push({ command, cwd, shell: options.shell }); void Promise.resolve(this.reply(command, cwd)).then((reply) => { for (const [file, content] of Object.entries(reply.writes ?? {})) this.write(file, content); callback(reply.error ? new Error(`exit 1: ${command}`) : null, reply.stdout ?? '', reply.stderr ?? ''); @@ -129,7 +145,8 @@ class FakeProblemMatcher { for (const id of [...this.entries.keys()]) if (!keep(id)) this.entries.delete(id); } clear() { this.entries.clear(); } - dispose() { } + disposed = false; + dispose() { this.disposed = true; } } function diagnostic(file: string, line: number, severity: number, message: string): FileDiagnostic { @@ -294,12 +311,49 @@ suite('CeedlingEngine.discover', () => { assert.ok(logs.some((l) => l.level === 'warn' && /test_missing/.test(l.message))); }); + test('dumpconfig output that is not YAML is a load error, with the parse error logged', async () => { + const world = new FakeCeedling(); + world.resolvedConfig = ':plugins: [unclosed\n'; + const logs: LogCall[] = []; + await assert.rejects(makeEngine(world, { logs }).discover(), (e: unknown) => e instanceof CeedlingLoadError && /dumpconfig/.test(e.message)); + assert.ok(logs.some((l) => l.level === 'error' && /could not parse dumpconfig output/.test(l.message))); + }); + + test('a failed test listing is logged, and leaves the project without test files', async () => { + const world = new FakeCeedling(); + world.handlers.push((command) => /^ceedling files:test/.test(command) ? { error: true, stderr: 'boom' } : undefined); + const logs: LogCall[] = []; + const { projects } = await discovered(world, { logs }); + assert.deepStrictEqual(projects[0].files, []); + assert.ok(logs.some((l) => l.level === 'error' && /files:test/.test(l.message))); + }); + + test("a test file's discovery warnings are logged, naming the file", async () => { + const world = new FakeCeedling(); + world.write(`${WS}/test/test_calc.c`, 'TEST_RANGE([0, 2, 0])\nvoid test_z(int x) {}\n'); + const logs: LogCall[] = []; + await discovered(world, { logs }); + assert.ok(logs.some((l) => l.level === 'warn' && /test\/test_calc\.c/.test(l.message) && /zero step/.test(l.message))); + }); + + test('a test file prefix that is not a valid pattern leaves file labels plain, with a warning', async () => { + const world = new FakeCeedling(); + world.resolvedConfig += ':project:\n :test_file_prefix: "test_("\n'; + world.testFiles = ['test/test_(calc.c']; + world.write(`${WS}/test/test_(calc.c`, 'void test_add(void) {}\n'); + const logs: LogCall[] = []; + const { file } = await discovered(world, { logs }); + assert.strictEqual(file.label, 'test/test_(calc.c'); + assert.strictEqual(file.functions[0].label, 'add'); + assert.ok(logs.some((l) => l.level === 'warn' && /Pretty labels are unavailable/.test(l.message))); + }); + test('the project file is watched even when discovery fails', async () => { const world = new FakeCeedling(); world.resolvedConfig = ':plugins:\n :enabled: []\n'; const engine = makeEngine(world); await assert.rejects(engine.discover()); - assert.ok(engine.configurationFiles().includes(`${WS}/project.yml`)); + assert.ok((await engine.configurationFiles()).includes(`${WS}/project.yml`)); }); // A failed rediscovery must not leave runs without their --project/--mixin flags and cwd. @@ -326,6 +380,35 @@ suite('CeedlingEngine with Ceedling before 1.2.0', () => { assert.ok(![...world.files.keys()].some((f) => f.startsWith('/tmp/'))); }); + test('a failed dumpconfig is a load error, logged', async () => { + const world = new FakeCeedling(); + world.version = '1.1.10'; + world.handlers.push((command) => /^ceedling dumpconfig/.test(command) ? { error: true } : undefined); + const logs: LogCall[] = []; + await assert.rejects(makeEngine(world, { logs }).discover(), CeedlingLoadError); + assert.ok(logs.some((l) => l.level === 'error' && /'ceedling dumpconfig' failed/.test(l.message))); + }); + + test('a dumpconfig that writes no file is a load error, logged', async () => { + const world = new FakeCeedling(); + world.version = '1.1.10'; + world.handlers.push((command) => /^ceedling dumpconfig/.test(command) ? {} : undefined); + const logs: LogCall[] = []; + await assert.rejects(makeEngine(world, { logs }).discover(), CeedlingLoadError); + assert.ok(logs.some((l) => l.level === 'error' && /could not read dumpconfig output/.test(l.message))); + }); + + test("a temporary file that can't be removed doesn't fail discovery", async () => { + const world = new FakeCeedling(); + world.version = '1.1.10'; + world.handlers.push((command) => { + const dump = /^ceedling dumpconfig (\S+)/.exec(command); + if (dump) world.undeletable.add(dump[1]); + return undefined; + }); + assert.strictEqual((await makeEngine(world).discover()).length, 1); + }); + test("test tasks name the file alone and don't force a rerun", async () => { const world = new FakeCeedling(); world.version = '1.1.10'; @@ -359,7 +442,7 @@ suite('CeedlingEngine with Ceedling 1.2.0', () => { const { engine, projects } = await discovered(world); assert.strictEqual(projects.length, 1); assert.match(world.commands[2].command, new RegExp(`--project ceedling\\.yml --mixin ${BUNDLED_MIXIN}$`)); - assert.ok(engine.configurationFiles().includes(`${WS}/ceedling.yml`)); + assert.ok((await engine.configurationFiles()).includes(`${WS}/ceedling.yml`)); }); // Ceedling itself refuses this ambiguity, but only when no --project names the file. @@ -376,7 +459,7 @@ suite('CeedlingEngine with Ceedling 1.2.0', () => { world.write(`${WS}/fw/project.yml`, ':project: {}\n'); world.write(`${WS}/fw/ci.yml`, ':project: {}\n'); const engine = makeEngine(world, { settings: { projects: [{ path: 'fw/ci.yml', debugLaunchConfig: 'dbg' }] } }); - assert.deepStrictEqual(engine.configurationFiles().sort(), [`${WS}/fw/ceedling.yml`, `${WS}/fw/ci.yml`, `${WS}/fw/project.yml`]); + assert.deepStrictEqual((await engine.configurationFiles()).sort(), [`${WS}/fw/ceedling.yml`, `${WS}/fw/ci.yml`, `${WS}/fw/project.yml`]); }); // 1.2.0 nests each executable under its test file's directory, relative to the test path. @@ -433,6 +516,28 @@ suite('CeedlingEngine runs', () => { assert.match(String(errored[0][2]), /reported no result/); }); + test('an ignored test is skipped', async () => { + const world = new FakeCeedling(); + const { engine, file } = await discovered(world); + world.handlers.push((command) => /^ceedling test:/.test(command) + ? { writes: world.reportWrite(cppunit({ passed: ['test/test_calc.c::test_add'], ignored: ['test/test_calc.c::test_sub'] })) } + : undefined); + const reporter = recordingReporter(); + await engine.runFunction('default', file, file.functions[1], reporter, token()); + assert.deepStrictEqual(reporter.of('skipped').map((c) => c[1]), ['test/test_calc.c::test_sub']); + }); + + test('results matching no discovered test are logged', async () => { + const world = new FakeCeedling(); + const logs: LogCall[] = []; + const { engine, file } = await discovered(world, { logs }); + world.handlers.push((command) => /^ceedling test:/.test(command) + ? { writes: world.reportWrite(cppunit({ passed: ['test/test_calc.c::test_add', 'test/test_calc.c::test_gone'] })) } + : undefined); + await engine.runFunction('default', file, file.functions[0], recordingReporter(), token()); + assert.ok(logs.some((l) => l.level === 'warn' && /test_gone/.test(l.message))); + }); + test('a failure without a location still reports, without a line', async () => { const world = new FakeCeedling(); const { engine, file } = await discovered(world); @@ -472,6 +577,18 @@ suite('CeedlingEngine runs', () => { assert.strictEqual(reporter.of('errored').length, 6); }); + test("a stale report that can't be deleted is warned about, and the run goes on", async () => { + const world = new FakeCeedling(); + const logs: LogCall[] = []; + const { engine, file } = await discovered(world, { logs }); + const stale = `${WS}/build/artifacts/test/cppunit_tests_report.xml`; + world.write(stale, cppunit({})); + world.undeletable.add(stale); + await engine.runFile('default', file, recordingReporter(), token()); + assert.ok(logs.some((l) => l.level === 'warn' && l.message.includes(stale))); + assert.strictEqual(world.commandsMatching(/^ceedling test:/).length, 1); + }); + test('a cancelled run marks its tests skipped, not errored', async () => { const world = new FakeCeedling(); const { engine, file } = await discovered(world); @@ -543,6 +660,15 @@ suite('CeedlingEngine runs', () => { assert.match(world.commandsMatching(/^ceedling test:/)[0], / --verbosity debug$/); }); + test("development mode logs a run's stdout and stderr", async () => { + const world = new FakeCeedling(); + const logs: LogCall[] = []; + const { engine, file } = await discovered(world, { logs, developmentMode: true }); + world.handlers.push((command) => /^ceedling test:/.test(command) ? { stdout: 'OUT', stderr: 'ERR' } : undefined); + await engine.runFile('default', file, recordingReporter(), token()); + assert.ok(logs.some((l) => l.level === 'debug' && /^stdout:\nOUT\nstderr:\nERR$/.test(l.message))); + }); + test('runs wait for one another', async () => { const world = new FakeCeedling(); const { engine, file } = await discovered(world); @@ -618,6 +744,19 @@ suite('CeedlingEngine.compileForDebug', () => { assert.deepStrictEqual(reporter.of('failed'), []); }); + test('a cancelled build marks its tests skipped and returns no executable', async () => { + const world = new FakeCeedling(); + const { engine, file } = await discovered(world); + const cancel = token(); + world.handlers.push((command) => /^ceedling test:/.test(command) + ? new Promise((resolve) => setImmediate(() => { cancel.fire(); resolve({ error: true }); })) + : undefined); + const reporter = recordingReporter(); + assert.strictEqual(await engine.compileForDebug('default', file, reporter, cancel), undefined); + assert.deepStrictEqual(reporter.of('skipped').map((c) => c[1]), leafIds(file.functions)); + assert.deepStrictEqual(reporter.of('errored'), []); + }); + test('waits for a run in progress', async () => { const world = new FakeCeedling(); const { engine, file } = await discovered(world); @@ -680,16 +819,16 @@ suite('CeedlingEngine against real Ceedling output', () => { suite('CeedlingEngine for watchers and editing', () => { // A project file that doesn't exist yet is watched too, so creating one starts discovery. - test("configuration files include absent default project files and each project's file-path mixins", () => { + test("configuration files include absent default project files and each project's file-path mixins", async () => { const world = new FakeCeedling(); world.directories.add(`${WS}/fw`); const engine = makeEngine(world, { settings: { projects: [{ path: 'fw', debugLaunchConfig: 'dbg', mixins: ['gcc', 'mixins/cov.yml'] }] } }); - assert.deepStrictEqual(engine.configurationFiles().sort(), [`${WS}/fw/ceedling.yml`, `${WS}/fw/mixins/cov.yml`, `${WS}/fw/project.yml`]); + assert.deepStrictEqual((await engine.configurationFiles()).sort(), [`${WS}/fw/ceedling.yml`, `${WS}/fw/mixins/cov.yml`, `${WS}/fw/project.yml`]); }); - test('configuration files are empty, not an error, when settings name a missing directory', () => { + test('configuration files are empty, not an error, when settings name a missing directory', async () => { const engine = makeEngine(new FakeCeedling(), { settings: { projects: [{ path: 'nope', debugLaunchConfig: 'dbg' }] } }); - assert.deepStrictEqual(engine.configurationFiles(), []); + assert.deepStrictEqual((await engine.configurationFiles()), []); }); test('a discovered test file is found by its absolute path', async () => { @@ -737,4 +876,244 @@ suite('CeedlingEngine for watchers and editing', () => { assert.ok(world.commands.length > 0); assert.ok(world.commands.every((c) => c.command.startsWith('bundle exec ceedling '))); }); + + test('a blank Ceedling command means ceedling', async () => { + const world = new FakeCeedling(); + await discovered(world, { settings: { ceedlingCommand: ' ' } }); + assert.ok(world.commands.every((c) => c.command.startsWith('ceedling '))); + }); + + // VS Code settings can hold the string "null" where the JSON null was meant. + test('the configured shell runs every invocation, and "null" means the default shell', async () => { + const configured = new FakeCeedling(); + await discovered(configured, { settings: { shellPath: '/bin/bash' } }); + assert.ok(configured.commands.every((c) => c.shell === '/bin/bash')); + const literalNull = new FakeCeedling(); + await discovered(literalNull, { settings: { shellPath: 'null' } }); + assert.ok(literalNull.commands.every((c) => c.shell === undefined)); + }); +}); + +suite('CeedlingEngine lookups and lifecycle', () => { + test("a project's debug configuration and directory are found by its key", async () => { + const world = new FakeCeedling(); + world.directories.add(`${WS}/fw`); + world.write(`${WS}/fw/project.yml`, ':project: {}\n'); + const { engine } = await discovered(world, { settings: { projects: [{ path: 'fw', debugLaunchConfig: 'gdb (fw)' }] } }); + assert.strictEqual(engine.getDebugLaunchConfig('fw'), 'gdb (fw)'); + assert.strictEqual(engine.getProjectAbsPath('fw'), `${WS}/fw`); + }); + + test('an unknown project key is an error naming it', async () => { + const { engine, file } = await discovered(new FakeCeedling()); + assert.throws(() => engine.getProjectAbsPath('nope'), /Unknown Ceedling project 'nope'/); + await assert.rejects(engine.runFile('nope', file, recordingReporter(), token()), /Unknown Ceedling project 'nope'/); + }); + + test('rediscovering a file in an unknown project finds nothing', async () => { + const { engine, file } = await discovered(new FakeCeedling()); + assert.strictEqual(await engine.rediscoverFile('nope', file.id, ''), undefined); + }); + + test('cancel stops every running Ceedling process', async () => { + const world = new FakeCeedling(); + const { engine, file } = await discovered(world); + let release!: () => void; + world.handlers.push((command) => /^ceedling test:/.test(command) ? new Promise((resolve) => { release = () => resolve({}); }) : undefined); + const run = engine.runFile('default', file, recordingReporter(), token()); + await new Promise((resolve) => setImmediate(resolve)); + engine.cancel(); + assert.strictEqual(world.killed.length, 1); + release(); + await run; + }); + + test('dispose stops running processes and disposes the problem matcher', async () => { + const world = new FakeCeedling(); + const problems = new FakeProblemMatcher(); + const { engine, file } = await discovered(world, { problems }); + let release!: () => void; + world.handlers.push((command) => /^ceedling test:/.test(command) ? new Promise((resolve) => { release = () => resolve({}); }) : undefined); + const run = engine.runFile('default', file, recordingReporter(), token()); + await new Promise((resolve) => setImmediate(resolve)); + engine.dispose(); + assert.strictEqual(world.killed.length, 1); + assert.strictEqual(problems.disposed, true); + release(); + await run; + }); + + // Default dependencies: the real filesystem and process runner. + test('with only its required dependencies, a folder without a project file on disk is no project', async () => { + const folder = fs.mkdtempSync(path.join(os.tmpdir(), 'ceedling-engine-')); + try { + const engine = new CeedlingEngine( + { uri: { fsPath: folder, toString: () => `file://${folder}` }, name: 'empty', index: 0 } as unknown as vscode.WorkspaceFolder, + fakeLogger(), false, + { + configProvider: () => ({ get: (_key: string, fallback?: unknown) => fallback }) as unknown as vscode.WorkspaceConfiguration, + problemMatcher: new FakeProblemMatcher() as unknown as ProblemMatcher, + bundledMixinPath: BUNDLED_MIXIN, + }, + ); + assert.deepStrictEqual(await engine.discover(), []); + } finally { + fs.rmSync(folder, { recursive: true, force: true }); + } + }); +}); + +suite('nodeFileSystem', () => { + let folder: string; + setup(() => { folder = fs.mkdtempSync(path.join(os.tmpdir(), 'ceedling-fs-')); }); + teardown(() => { fs.rmSync(folder, { recursive: true, force: true }); }); + + test('a file exists, has a modification time, reads back, and is gone once unlinked', async () => { + const file = path.join(folder, 'project.yml'); + fs.writeFileSync(file, ':project: {}\n'); + assert.strictEqual(await nodeFileSystem.exists(file), true); + assert.strictEqual(await nodeFileSystem.isDirectory(file), false); + assert.strictEqual(typeof await nodeFileSystem.mtimeMs(file), 'number'); + assert.strictEqual(await nodeFileSystem.readFile(file), ':project: {}\n'); + await nodeFileSystem.unlink(file); + assert.strictEqual(await nodeFileSystem.exists(file), false); + }); + + test('a directory is a directory', async () => { + assert.strictEqual(await nodeFileSystem.isDirectory(folder), true); + }); + + test('a missing path does not exist, is no directory, and has no modification time, without an error', async () => { + const missing = path.join(folder, 'missing'); + assert.strictEqual(await nodeFileSystem.exists(missing), false); + assert.strictEqual(await nodeFileSystem.isDirectory(missing), false); + assert.strictEqual(await nodeFileSystem.mtimeMs(missing), undefined); + }); +}); + +// Each Ceedling start costs a Ruby startup, so discovery overlaps work it can. Two Ceedling +// processes in one project directory could collide on its build files, so that never overlaps. +suite('CeedlingEngine concurrent discovery', () => { + function twoProjects(world: FakeCeedling, a: string, b: string) { + for (const dir of new Set([a, b].map((p) => p.replace(/\/[^/]*\.yml$/, '')))) { + world.directories.add(`${WS}/${dir}`); + world.write(`${WS}/${dir}/project.yml`, ':project: {}\n'); + } + for (const file of [a, b].filter((p) => p.endsWith('.yml'))) world.write(`${WS}/${file}`, ':project: {}\n'); + return { projects: [{ path: a, debugLaunchConfig: 'dbg' }, { path: b, debugLaunchConfig: 'dbg' }] }; + } + const settle = () => new Promise((resolve) => setImmediate(resolve)); + + test('projects in different directories are checked at the same time', async () => { + const world = new FakeCeedling(); + let release!: () => void; + const held = new Promise((resolve) => { release = () => resolve({ stdout: 'Ceedling => 1.2.0\n' }); }); + world.handlers.push((command) => /^ceedling version/.test(command) ? held : undefined); + const discovery = makeEngine(world, { settings: twoProjects(world, 'a', 'b') }).discover(); + await settle(); + assert.deepStrictEqual(world.commands.filter((c) => /version/.test(c.command)).map((c) => c.cwd).sort(), [`${WS}/a`, `${WS}/b`]); + release(); + assert.strictEqual((await discovery).length, 2); + }); + + test('projects sharing a directory are loaded one after the other', async () => { + const world = new FakeCeedling(); + let release!: () => void; + const held = new Promise((resolve) => { release = () => resolve({ stdout: 'Ceedling => 1.2.0\n' }); }); + world.handlers.push((command) => /^ceedling version/.test(command) && world.commandsMatching(/version/).length === 1 ? held : undefined); + const discovery = makeEngine(world, { settings: twoProjects(world, 'fw/ci.yml', 'fw/other.yml') }).discover(); + await settle(); + assert.strictEqual(world.commandsMatching(/version/).length, 1); + release(); + assert.strictEqual((await discovery).length, 2); + }); + + test('load errors name the first failing project in settings order, however the checks finish', async () => { + const world = new FakeCeedling(); + world.handlers.push((command, cwd) => /^ceedling version/.test(command) + ? (cwd.endsWith('/a') + ? new Promise((resolve) => setTimeout(() => resolve({ error: true }), 20)) + : { error: true }) + : undefined); + await assert.rejects(makeEngine(world, { settings: twoProjects(world, 'a', 'b') }).discover(), + (e: unknown) => e instanceof CeedlingLoadError && /project 'a'/.test(e.message)); + }); + + test('several failed configurations are reported together, in settings order', async () => { + const world = new FakeCeedling(); + world.handlers.push((command, cwd) => /dumpconfig/.test(command) + ? (cwd.endsWith('/a') + ? new Promise((resolve) => setTimeout(() => resolve({ error: true }), 20)) + : { error: true }) + : undefined); + await assert.rejects(makeEngine(world, { settings: twoProjects(world, 'a', 'b') }).discover(), + (e: unknown) => e instanceof CeedlingLoadError && e.message.indexOf("'a'") < e.message.indexOf("'b'") && e.message.includes("'a'")); + }); + + // Every Ceedling command that loads a project rewrites the Unity copy in its build directory, so + // a configuration check during a build in the same directory could corrupt that build. + test("a rediscovery's configuration check waits for a build in the same directory", async () => { + const world = new FakeCeedling(); + const { engine, file } = await discovered(world); + let release!: () => void; + world.handlers.push((command) => /^ceedling test:/.test(command) ? new Promise((resolve) => { release = () => resolve({}); }) : undefined); + const run = engine.runFile('default', file, recordingReporter(), token()); + await settle(); + const before = world.commandsMatching(/dumpconfig/).length; + const rediscovery = engine.discover(); + await settle(); + await settle(); + assert.strictEqual(world.commandsMatching(/dumpconfig/).length, before, 'no dumpconfig while the build runs'); + release(); + await Promise.all([run, rediscovery]); + assert.strictEqual(world.commandsMatching(/dumpconfig/).length, before + 1); + }); + + test('builds in different directories may run at once', async () => { + const world = new FakeCeedling(); + world.testFiles = ['test/test_calc.c']; + for (const dir of ['a', 'b']) world.write(`${WS}/${dir}/test/test_calc.c`, 'void test_add(void) {}\n'); + const { engine, projects } = await discovered(world, { settings: twoProjects(world, 'a', 'b') }); + const held: Array<() => void> = []; + world.handlers.push((command) => /^ceedling test:/.test(command) ? new Promise((resolve) => { held.push(() => resolve({})); }) : undefined); + const runs = projects.map((p) => engine.runFile(p.projectKey, p.files[0], recordingReporter(), token())); + await settle(); + assert.deepStrictEqual(world.commands.filter((c) => /^ceedling test:/.test(c.command)).map((c) => c.cwd).sort(), [`${WS}/a`, `${WS}/b`]); + held.forEach((release) => release()); + await Promise.all(runs); + }); + + // Many watcher events at once, e.g. from a branch checkout, must not exhaust file descriptors. + test('reads stay within one shared limit across rediscoveries and reparses', async () => { + const world = new FakeCeedling(); + world.testFiles = Array.from({ length: 40 }, (_, i) => `test/test_m${i}.c`); + for (const file of world.testFiles) world.write(`${WS}/${file}`, 'void test_a(void) {}\n'); + const { engine } = await discovered(world); + let open!: () => void; + world.readGate = new Promise((resolve) => { open = resolve; }); + world.maxReadsInFlight = 0; + const work = [engine.discover(), ...world.testFiles.map((file) => engine.rediscoverFile('default', file))]; + await settle(); + await settle(); + open(); + await Promise.all(work); + assert.ok(world.maxReadsInFlight <= 16, `at most 16 reads at once, saw ${world.maxReadsInFlight}`); + assert.ok(world.maxReadsInFlight > 1, 'reads still overlap'); + }); + + test('test files are read at the same time, and keep their listed order', async () => { + const world = new FakeCeedling(); + world.testFiles = ['test/test_calc.c', 'test/test_more.c']; + world.write(`${WS}/test/test_more.c`, 'void test_more(void) {}\n'); + let open!: () => void; + world.readGate = new Promise((resolve) => { open = resolve; }); + const discovery = makeEngine(world).discover(); + // eslint-disable-next-line no-await-in-loop -- polls until discovery reaches listing + while (!world.commandsMatching(/files:test/).length) await settle(); + await settle(); + assert.deepStrictEqual(world.reads.filter((r) => r.includes('/test/')), [`${WS}/test/test_calc.c`, `${WS}/test/test_more.c`]); + open(); + const [project] = await discovery; + assert.deepStrictEqual(project.files.map((f) => f.id), ['test/test_calc.c', 'test/test_more.c']); + }); }); diff --git a/tests/unit/ceedlingProcess.test.ts b/tests/unit/ceedlingProcess.test.ts index 2fc7255..c601899 100644 --- a/tests/unit/ceedlingProcess.test.ts +++ b/tests/unit/ceedlingProcess.test.ts @@ -1,6 +1,8 @@ import * as assert from 'assert'; -import type * as child_process from 'child_process'; -import { buildCommandLine, type Cancellation, type CeedlingInvocation, CeedlingRunner, ceedlingEnvironment, descendantsOf, parseProcStatParent, quoteIfNeeded } from '../../src/ceedlingProcess'; +import * as child_process from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import { buildCommandLine, type Cancellation, type CeedlingInvocation, CeedlingRunner, ceedlingEnvironment, descendantsOf, killProcessTree, parseProcStatParent, quoteIfNeeded } from '../../src/ceedlingProcess'; type ExecCall = { command: string, options: child_process.ExecOptions, finish: (error: Error | null, stdout: string, stderr: string) => void }; @@ -152,6 +154,96 @@ suite('CeedlingRunner', () => { runner.cancelAll(); assert.deepStrictEqual(killed, [], 'a finished process is not tracked as live'); }); + + test('with no doubles, runs a real process in the given directory with Ceedling\'s environment', async () => { + const cwd = fs.realpathSync(os.tmpdir()); + const script = '"console.log(process.env.CEEDLING_DECORATORS + process.cwd())"'; + const result = await new CeedlingRunner().run(invocation({ command: quoteIfNeeded(process.execPath), args: ['-e', script], cwd })); + assert.strictEqual(result.error, null); + assert.strictEqual(result.stdout.trim(), `false${cwd}`); + }); +}); + +// Above Linux's largest pid, and odd, as no Windows pid is: no process ever has it. +const NONEXISTENT_PID = 4194305; + +// Gone, or a zombie waiting for a parent to reap it: either way, no longer running. +function running(pid: number): boolean { + if (process.platform === 'linux') { + try { + return !/^\S+ \(.*\) Z/.test(fs.readFileSync(`/proc/${pid}/stat`, 'utf8')); + } catch { + return false; + } + } + try { + process.kill(pid, 0); + return true; + } catch (e) { + return (e as NodeJS.ErrnoException).code === 'EPERM'; + } +} + +async function stopped(pid: number, timeoutMs = 10000): Promise { + for (const deadline = Date.now() + timeoutMs; Date.now() < deadline;) { + if (!running(pid)) return true; + // eslint-disable-next-line no-await-in-loop -- polling + await new Promise((resolve) => setTimeout(resolve, 50)); + } + return false; +} + +// A Node process that starts a child of its own, as Ceedling starts compilers and test executables. +async function processTree(): Promise<{ root: child_process.ChildProcess, child: number }> { + const idle = 'setInterval(() => {}, 1000)'; + const script = `const c = require('child_process').spawn(process.execPath, ['-e', ${JSON.stringify(idle)}], { stdio: 'ignore' }); console.log(c.pid); ${idle}`; + const root = child_process.spawn(process.execPath, ['-e', script], { stdio: ['ignore', 'pipe', 'ignore'] }); + const child = await new Promise((resolve, reject) => { + root.stdout.once('data', (data: Buffer) => resolve(Number(String(data).trim()))); + root.once('error', reject); + }); + return { root, child }; +} + +suite('killProcessTree', () => { + test('stops a process and the processes it started', async function () { + this.timeout(20000); + const { root, child } = await processTree(); + try { + killProcessTree(root.pid!); + assert.ok(await stopped(root.pid!), 'the root process is still running'); + assert.ok(await stopped(child), 'its child process is still running'); + } finally { + for (const pid of [root.pid!, child]) if (running(pid)) process.kill(pid); + } + }); + + test('a process that has already exited is no error', () => { + assert.doesNotThrow(() => killProcessTree(NONEXISTENT_PID)); + }); + + // tree-kill depends on pgrep (macOS) or taskkill (Windows), either of which could fail. + test('where tree-kill fails, the root process is stopped alone', async function () { + this.timeout(20000); + const root = child_process.spawn(process.execPath, ['-e', 'setInterval(() => {}, 1000)'], { stdio: 'ignore' }); + const calls: unknown[][] = []; + const failing = (...args: unknown[]) => { + calls.push(args.slice(0, 2)); + (args[2] as (error?: Error) => void)(new Error('pgrep: not found')); + }; + try { + killProcessTree(root.pid!, { platform: 'darwin', treeKill: failing }); + assert.deepStrictEqual(calls, [[root.pid, 'SIGTERM']]); + assert.ok(await stopped(root.pid!), 'the root process is still running'); + } finally { + if (running(root.pid!)) process.kill(root.pid!); + } + }); + + test('where tree-kill fails for a process that has already exited, that is no error', () => { + const failing = (_pid: number, _signal: string, callback: (error?: Error) => void) => callback(new Error('no such process')); + assert.doesNotThrow(() => killProcessTree(NONEXISTENT_PID, { platform: 'darwin', treeKill: failing })); + }); }); // Linux process-tree killing reads /proc instead of spawning `ps`, which minimal containers lack. diff --git a/tests/unit/ceedlingProjectConfig.test.ts b/tests/unit/ceedlingProjectConfig.test.ts index 4f04100..5fa92fb 100644 --- a/tests/unit/ceedlingProjectConfig.test.ts +++ b/tests/unit/ceedlingProjectConfig.test.ts @@ -10,60 +10,60 @@ function dumpconfigFixture(name: string): unknown { return yaml.safeLoad(fs.readFileSync(path.resolve(fixturesRoot, 'dumpconfig', name), 'utf8')); } -const anyDirectory = () => true; -const noFiles = () => false; +const anyDirectory = async () => true; +const noFiles = async () => false; // A filesystem where exactly the listed files exist. -const filesAt = (...files: string[]) => (p: string) => files.includes(p); +const filesAt = (...files: string[]) => async (p: string) => files.includes(p); suite('projectsFromSettings', () => { - test('with no projects configured, one default project sits at the workspace root', () => { - const projects = projectsFromSettings({ projects: [], mixins: ['gcc'] }, '/ws', 'linux', anyDirectory, noFiles); + test('with no projects configured, one default project sits at the workspace root', async () => { + const projects = await projectsFromSettings({ projects: [], mixins: ['gcc'] }, '/ws', 'linux', anyDirectory, noFiles); assert.deepStrictEqual(projects, [{ key: 'default', absPath: '/ws', projectFileName: 'project.yml', ymlFileName: 'project.yml', ambiguousProjectFile: false, debugLaunchConfig: 'ceedling', mixins: ['gcc'], }]); }); - test('a directory path is keyed by that path and uses project.yml', () => { - const [project] = projectsFromSettings({ projects: [{ path: 'fw/app', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); + test('a directory path is keyed by that path and uses project.yml', async () => { + const [project] = await projectsFromSettings({ projects: [{ path: 'fw/app', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); assert.deepStrictEqual(project, { key: 'fw/app', absPath: '/ws/fw/app', projectFileName: 'project.yml', ymlFileName: 'project.yml', ambiguousProjectFile: false, debugLaunchConfig: 'dbg', mixins: [], }); }); - test('an alternate yml path is keyed by its file name and resolves to its directory', () => { - const [project] = projectsFromSettings({ projects: [{ path: 'fw/ci.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); + test('an alternate yml path is keyed by its file name and resolves to its directory', async () => { + const [project] = await projectsFromSettings({ projects: [{ path: 'fw/ci.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); assert.strictEqual(project.key, 'ci'); assert.strictEqual(project.ymlFileName, 'ci.yml'); assert.strictEqual(project.absPath, '/ws/fw'); }); - test('a yml path at the workspace root resolves to the root', () => { - const [project] = projectsFromSettings({ projects: [{ path: 'ci.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); + test('a yml path at the workspace root resolves to the root', async () => { + const [project] = await projectsFromSettings({ projects: [{ path: 'ci.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); assert.strictEqual(project.absPath, '/ws'); }); - test('an explicit project.yml path keeps the configured path as its key', () => { - const [project] = projectsFromSettings({ projects: [{ path: 'fw/project.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); + test('an explicit project.yml path keeps the configured path as its key', async () => { + const [project] = await projectsFromSettings({ projects: [{ path: 'fw/project.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); assert.strictEqual(project.key, 'fw/project.yml'); assert.strictEqual(project.ymlFileName, 'project.yml'); assert.strictEqual(project.absPath, '/ws/fw'); }); - test('backslash separators in a yml path are understood', () => { - const [project] = projectsFromSettings({ projects: [{ path: 'fw\\ci.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); + test('backslash separators in a yml path are understood', async () => { + const [project] = await projectsFromSettings({ projects: [{ path: 'fw\\ci.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); assert.strictEqual(project.ymlFileName, 'ci.yml'); assert.strictEqual(project.absPath, '/ws/fw'); }); - test('a name overrides the derived key', () => { - const [project] = projectsFromSettings({ projects: [{ path: 'fw/ci.yml', debugLaunchConfig: 'dbg', name: 'Firmware' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); + test('a name overrides the derived key', async () => { + const [project] = await projectsFromSettings({ projects: [{ path: 'fw/ci.yml', debugLaunchConfig: 'dbg', name: 'Firmware' }], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); assert.strictEqual(project.key, 'Firmware'); }); - test("a project's own mixins replace the workspace list; an explicit [] opts out of it", () => { - const projects = projectsFromSettings({ + test("a project's own mixins replace the workspace list; an explicit [] opts out of it", async () => { + const projects = await projectsFromSettings({ projects: [ { path: 'a', debugLaunchConfig: 'dbg' }, { path: 'b', debugLaunchConfig: 'dbg', mixins: ['clang'] }, @@ -74,50 +74,50 @@ suite('projectsFromSettings', () => { assert.deepStrictEqual(projects.map((p) => p.mixins), [['gcc'], ['clang'], []]); }); - test('a path that is not a directory is a load error naming it', () => { - assert.throws( - () => projectsFromSettings({ projects: [{ path: 'missing', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', () => false, noFiles), + test('a path that is not a directory is a load error naming it', async () => { + await assert.rejects( + projectsFromSettings({ projects: [{ path: 'missing', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', async () => false, noFiles), (e: unknown) => e instanceof CeedlingLoadError && /\/ws\/missing/.test(e.message), ); }); // Gcov's XML report generation needs an uppercase drive letter on Windows. - test('on Windows the workspace drive letter is uppercased', () => { - const [project] = projectsFromSettings({ projects: [], mixins: [] }, 'c:\\ws', 'win32', anyDirectory, noFiles); + test('on Windows the workspace drive letter is uppercased', async () => { + const [project] = await projectsFromSettings({ projects: [], mixins: [] }, 'c:\\ws', 'win32', anyDirectory, noFiles); assert.match(project.absPath, /^C:/); }); - test('the settings objects are not modified', () => { + test('the settings objects are not modified', async () => { const setting = { path: 'fw/ci.yml', debugLaunchConfig: 'dbg' }; - projectsFromSettings({ projects: [setting], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); + await projectsFromSettings({ projects: [setting], mixins: [] }, '/ws', 'linux', anyDirectory, noFiles); assert.deepStrictEqual(setting, { path: 'fw/ci.yml', debugLaunchConfig: 'dbg' }); }); }); // Ceedling 1.2.0 also loads ceedling.yml as a project's default project file. suite('projectsFromSettings: the default project file', () => { - test('a directory holding only ceedling.yml uses it', () => { - const [project] = projectsFromSettings({ projects: [], mixins: [] }, '/ws', 'linux', anyDirectory, filesAt('/ws/ceedling.yml')); + test('a directory holding only ceedling.yml uses it', async () => { + const [project] = await projectsFromSettings({ projects: [], mixins: [] }, '/ws', 'linux', anyDirectory, filesAt('/ws/ceedling.yml')); assert.strictEqual(project.projectFileName, 'ceedling.yml'); assert.strictEqual(project.ymlFileName, 'ceedling.yml'); assert.strictEqual(project.ambiguousProjectFile, false); }); - test('a directory holding both uses project.yml and flags the ambiguity', () => { - const [project] = projectsFromSettings({ projects: [], mixins: [] }, '/ws', 'linux', anyDirectory, filesAt('/ws/project.yml', '/ws/ceedling.yml')); + test('a directory holding both uses project.yml and flags the ambiguity', async () => { + const [project] = await projectsFromSettings({ projects: [], mixins: [] }, '/ws', 'linux', anyDirectory, filesAt('/ws/project.yml', '/ws/ceedling.yml')); assert.strictEqual(project.projectFileName, 'project.yml'); assert.strictEqual(project.ambiguousProjectFile, true); }); - test('an alternate project file applies over a ceedling.yml base', () => { - const [project] = projectsFromSettings({ projects: [{ path: 'fw/ci.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, + test('an alternate project file applies over a ceedling.yml base', async () => { + const [project] = await projectsFromSettings({ projects: [{ path: 'fw/ci.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, filesAt('/ws/fw/ceedling.yml', '/ws/fw/ci.yml')); assert.strictEqual(project.projectFileName, 'ceedling.yml'); assert.strictEqual(project.ymlFileName, 'ci.yml'); }); - test('an explicit ceedling.yml path is the project file itself, keyed by its path', () => { - const [project] = projectsFromSettings({ projects: [{ path: 'fw/ceedling.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, + test('an explicit ceedling.yml path is the project file itself, keyed by its path', async () => { + const [project] = await projectsFromSettings({ projects: [{ path: 'fw/ceedling.yml', debugLaunchConfig: 'dbg' }], mixins: [] }, '/ws', 'linux', anyDirectory, filesAt('/ws/fw/ceedling.yml')); assert.strictEqual(project.key, 'fw/ceedling.yml'); assert.strictEqual(project.projectFileName, 'ceedling.yml'); diff --git a/tests/unit/concurrency.test.ts b/tests/unit/concurrency.test.ts new file mode 100644 index 0000000..e8d6228 --- /dev/null +++ b/tests/unit/concurrency.test.ts @@ -0,0 +1,125 @@ +import * as assert from 'assert'; +import { KeyedLock, Limiter, mapGrouped } from '../../src/concurrency'; + +// Work items that finish when the test says, recording how many ran at once. +function gate() { + const pending = new Map void>(); + const log = { started: [] as string[], inFlight: 0, maxInFlight: 0 }; + const work = (item: string) => new Promise((resolve, reject) => { + log.started.push(item); + log.inFlight++; + log.maxInFlight = Math.max(log.maxInFlight, log.inFlight); + pending.set(item, () => { + log.inFlight--; + if (item.startsWith('fail')) reject(new Error(item)); else resolve(item.toUpperCase()); + }); + }); + const finish = async (item: string) => { + pending.get(item)!(); + pending.delete(item); + await new Promise((resolve) => setImmediate(resolve)); + }; + return { work, finish, log, pending }; +} + +const settle = () => new Promise((resolve) => setImmediate(resolve)); + +suite('mapGrouped', () => { + const groupOf = (item: string) => item.charAt(0); + + test('runs different groups at once, and one item at a time within a group', async () => { + const g = gate(); + const done = mapGrouped(['x1', 'y1', 'x2'], groupOf, g.work); + await settle(); + assert.deepStrictEqual(g.log.started, ['x1', 'y1']); + await g.finish('x1'); + assert.deepStrictEqual(g.log.started, ['x1', 'y1', 'x2']); + await g.finish('x2'); + await g.finish('y1'); + assert.deepStrictEqual(await done, ['X1', 'Y1', 'X2']); + }); + + test('a failure stops its own and other groups from starting more, then rejects', async () => { + const g = gate(); + const done = mapGrouped(['xfail', 'y1', 'x2', 'y2'], (item) => item.startsWith('x') ? 'x' : 'y', (item) => g.work(item === 'xfail' ? 'fail-x' : item)); + const rejected = assert.rejects(done, /fail-x/); + await settle(); + await g.finish('fail-x'); + await g.finish('y1'); + await rejected; + assert.deepStrictEqual(g.log.started, ['fail-x', 'y1']); + }); +}); + +suite('KeyedLock', () => { + test('work under one key runs one at a time, in arrival order', async () => { + const g = gate(); + const lock = new KeyedLock(); + const runs = ['k1', 'k2', 'k3'].map((item) => lock.run('dir', () => g.work(item))); + await settle(); + assert.deepStrictEqual(g.log.started, ['k1']); + await g.finish('k1'); + assert.deepStrictEqual(g.log.started, ['k1', 'k2']); + await g.finish('k2'); + await g.finish('k3'); + assert.deepStrictEqual(await Promise.all(runs), ['K1', 'K2', 'K3']); + }); + + test('work under different keys runs at once', async () => { + const g = gate(); + const lock = new KeyedLock(); + const runs = [lock.run('a', () => g.work('a1')), lock.run('b', () => g.work('b1'))]; + await settle(); + assert.deepStrictEqual(g.log.started, ['a1', 'b1']); + await g.finish('a1'); + await g.finish('b1'); + await Promise.all(runs); + }); + + test('a failure releases the key for the next work, and reaches its own caller', async () => { + const g = gate(); + const lock = new KeyedLock(); + const failing = assert.rejects(lock.run('dir', () => g.work('fail1')), /fail1/); + const next = lock.run('dir', () => g.work('n1')); + await settle(); + await g.finish('fail1'); + await failing; + await g.finish('n1'); + assert.strictEqual(await next, 'N1'); + }); + + test('an idle key holds no state', async () => { + const lock = new KeyedLock(); + await lock.run('dir', async () => 1); + assert.strictEqual(lock.size, 0); + }); +}); + +suite('Limiter', () => { + test('runs at most its limit at once, across every caller, starting queued work in arrival order', async () => { + const g = gate(); + const limiter = new Limiter(2); + const runs = ['a', 'b', 'c', 'd'].map((item) => limiter.run(() => g.work(item))); + await settle(); + assert.deepStrictEqual(g.log.started, ['a', 'b']); + await g.finish('b'); + assert.deepStrictEqual(g.log.started, ['a', 'b', 'c']); + await g.finish('a'); + await g.finish('c'); + await g.finish('d'); + assert.deepStrictEqual(await Promise.all(runs), ['A', 'B', 'C', 'D']); + assert.strictEqual(g.log.maxInFlight, 2); + }); + + test('a failure frees its slot and reaches its own caller', async () => { + const g = gate(); + const limiter = new Limiter(1); + const failing = assert.rejects(limiter.run(() => g.work('fail1')), /fail1/); + const next = limiter.run(() => g.work('n1')); + await settle(); + await g.finish('fail1'); + await failing; + await g.finish('n1'); + assert.strictEqual(await next, 'N1'); + }); +}); diff --git a/tests/unit/problemMatching.test.ts b/tests/unit/problemMatching.test.ts index 54ffcef..cf5d258 100644 --- a/tests/unit/problemMatching.test.ts +++ b/tests/unit/problemMatching.test.ts @@ -154,6 +154,37 @@ suite('matchToProblem', () => { const matches = exec('^(.*):(\\w+):(.*)$', 'test/foo.c:NaN:whatever'); assert.strictEqual(matchToProblem(matches, 1, 3, 'error', '', 2, null, null, null), undefined); }); + + test('a configured last line and last column end the range', () => { + const matches = exec('^(.*):(\\d+)-(\\d+):(\\d+)-(\\d+): (.*)$', 'src/a.c:3-5:2-7: spans'); + const problem = matchToProblem(matches, 1, 6, 'warning', '', 2, 3, 4, 5); + assert.deepStrictEqual(problem && [problem.startLine, problem.startColumn, problem.endLine, problem.endColumn], [2, 1, 4, 6]); + }); + + test('a pattern without a line group places the problem at the top of the file', () => { + const matches = exec('^(.*): (.*)$', 'src/a.c: somewhere'); + const problem = matchToProblem(matches, 1, 2, 'info', '', null, null, null, null); + assert.deepStrictEqual(problem && [problem.startLine, problem.startColumn, problem.endLine, problem.endColumn], [0, 0, 0, 999]); + }); +}); + +suite('getPatternProblems', () => { + const pattern = (scanStdout: boolean, scanStderr: boolean): ProblemMatchingPattern => ({ + scanStdout, scanStderr, severity: 'error', filePrefix: '${projectPath}', + regexp: '^(\\S+\\.c):(\\d+): (.*)$', message: 3, file: 1, line: 2, lastLine: null, column: null, lastColumn: null, + }); + + test('scans only the streams the pattern names', () => { + const files = (p: ProblemMatchingPattern) => getPatternProblems('out.c:1: from stdout', 'err.c:1: from stderr', '/project', p).map((m) => m.message); + assert.deepStrictEqual(files(pattern(false, true)), ['from stderr']); + assert.deepStrictEqual(files(pattern(true, false)), ['from stdout']); + assert.deepStrictEqual(files(pattern(true, true)), ['from stdout', 'from stderr']); + }); + + test('${projectPath} in the file prefix becomes the project directory', () => { + const [problem] = getPatternProblems('', 'src/a.c:1: here', '/project', pattern(false, true)); + assert.strictEqual(problem.file, path.join('/project', 'src/a.c')); + }); }); suite('normalizePatterns', () => { @@ -185,6 +216,24 @@ suite('normalizePatterns', () => { assert.strictEqual(invalid.length, 1); }); + test('keeps each optional field that is configured, and falls back from an unknown severity', () => { + const { valid } = normalizePatterns([ + { regexp: 'x', message: 1, file: 2, scanStdout: true, scanStderr: false, severity: 'warning', filePrefix: '/p', line: 3, lastLine: 4, column: 5, lastColumn: 6 }, + { regexp: 'x', message: 1, file: 2, severity: 'fatal' }, + ]); + assert.deepStrictEqual(valid[0], { + scanStdout: true, scanStderr: false, severity: 'warning', filePrefix: '/p', + regexp: 'x', message: 1, file: 2, line: 3, lastLine: 4, column: 5, lastColumn: 6, + }); + assert.strictEqual(valid[1]?.severity, 'info'); + }); + + test('drops an entry that is not an object', () => { + const { valid, invalid } = normalizePatterns([null, 'gcc', 3]); + assert.deepStrictEqual(valid, []); + assert.deepStrictEqual(invalid, [null, 'gcc', 3]); + }); + test('drops a pattern whose required field is the wrong type', () => { const { valid, invalid } = normalizePatterns([ { regexp: '^(.*)$', message: '0', file: 0 }, // message is a string diff --git a/tests/unit/runPlan.test.ts b/tests/unit/runPlan.test.ts index 86720d0..883fc28 100644 --- a/tests/unit/runPlan.test.ts +++ b/tests/unit/runPlan.test.ts @@ -96,6 +96,10 @@ suite('buildRunPlan', () => { assert.deepStrictEqual(summarize(plan), ['function:test/test_a.c::test_1', 'function:test/test_a.c::test_2']); }); + test('excluding a function outside the selection changes nothing', () => { + assert.deepStrictEqual(summarize(buildRunPlan(projects, [fileTarget(fileA)], [fn(fileB, 'test_3'), fn(fileQ, 'test_4')])), ['file:test/test_a.c']); + }); + test('excluding every function of a file leaves nothing to run', () => { assert.deepStrictEqual(buildRunPlan(projects, [fileTarget(fileB)], [fn(fileB, 'test_3')]), []); }); diff --git a/tests/unit/unityTestDiscovery.test.ts b/tests/unit/unityTestDiscovery.test.ts index fc70dd0..9c57492 100644 --- a/tests/unit/unityTestDiscovery.test.ts +++ b/tests/unit/unityTestDiscovery.test.ts @@ -134,6 +134,15 @@ suite('discoverTests: TEST_RANGE', () => { assert.strictEqual(warnings.length, 1); assert.match(warnings[0], /test_z/); }); + + // Unity's generator fails on such a marker (`undefined method 'map' for nil`), so Ceedling + // cannot build the test. + test('a marker without a bracketed range yields no cases, and a warning', () => { + const { tests, warnings } = discoverTests('TEST_RANGE(1, 2, 3)\nvoid test_r(int x) {}\n', PARAMS_ON); + assert.deepStrictEqual(reportedNames(tests), ['test_r']); + assert.strictEqual(warnings.length, 1); + assert.match(warnings[0], /TEST_RANGE for test_r/); + }); }); suite('discoverTests: TEST_MATRIX', () => { @@ -149,6 +158,14 @@ suite('discoverTests: TEST_MATRIX', () => { test('string and character elements keep their quotes, commas inside them included', () => { assert.deepStrictEqual(namesIn(`TEST_MATRIX(["x,y", 'z'])\nvoid test_m(int x) {}\n`), ['test_m("x,y")', `test_m('z')`]); }); + + // Unity's generator fails on such a marker, as it does on a TEST_RANGE without a range. + test('a marker without lists yields no cases, and a warning', () => { + const { tests, warnings } = discoverTests('TEST_MATRIX(1, 2)\nvoid test_m(int x) {}\n', PARAMS_ON); + assert.deepStrictEqual(reportedNames(tests), ['test_m']); + assert.strictEqual(warnings.length, 1); + assert.match(warnings[0], /TEST_MATRIX for test_m/); + }); }); suite('discoverTests: line numbers', () => { @@ -217,6 +234,10 @@ suite('expandRubyRange', () => { ['1.0', '0.0', '-0.5', false, ['1.0', '0.5', '0.0']], ['1.0', '0.0', '-0.5', true, ['1.0', '0.5']], ['2', '0.0', '-1', false, ['2.0', '1.0', '0.0']], + ['1.0', '0.0', '0.5', true, []], + ['0.0', '0.5', '1.0', true, ['0.0']], + ['5.0', '5.0', '1', true, []], + ['0.0', '0.25', '0.1', true, ['0.0', '0.1', '0.2']], ]; for (const [start, end, step, excludeEnd, expected] of vectors) { const range = `${excludeEnd ? '<' : '['}${start}, ${end}, ${step}${excludeEnd ? '>' : ']'}`;