diff --git a/eslint.config.mjs b/eslint.config.mjs index 8e23af4..1bc102f 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -63,9 +63,11 @@ export default defineConfig( }, }, { - // Test doubles cast deliberately, and tests assert on values they know exist. + // Test doubles cast deliberately, implement async interfaces without awaiting, and tests + // assert on values they know exist. files: ['tests/**/*.ts'], rules: { + '@typescript-eslint/require-await': 'off', '@typescript-eslint/no-explicit-any': 'off', '@typescript-eslint/no-non-null-assertion': 'off', '@typescript-eslint/no-unsafe-argument': 'off', diff --git a/src/ceedlingEngine.ts b/src/ceedlingEngine.ts index c0eefc6..ba426df 100644 --- a/src/ceedlingEngine.ts +++ b/src/ceedlingEngine.ts @@ -27,24 +27,24 @@ import { parseFileListBullets, testCaseFilterMatchedExactly, } from './ceedlingOutputParsing'; -import { buildCommandLine, Cancellation, CeedlingResult, CeedlingRunner } from './ceedlingProcess'; +import { buildCommandLine, type Cancellation, type CeedlingResult, CeedlingRunner } from './ceedlingProcess'; import { ALTERNATE_DEFAULT_PROJECT_FILE, capabilitiesOf, - CeedlingCapabilities, + type CeedlingCapabilities, CeedlingLoadError, - CeedlingProject, + type CeedlingProject, DEFAULT_CPPUNIT_REPORT_FILENAME, DEFAULT_PROJECT_FILE, - ProjectSetting, - ProjectState, + type ProjectSetting, + type ProjectState, projectState, projectsFromSettings, } from './ceedlingProjectConfig'; import type { Logger } from './logger'; -import type { FileDiagnostic, ProblemMatcher, ProblemMatchingPattern } from './problemMatcher'; -import { allIds, CeedlingFileNode, CeedlingProjectNode, CeedlingTestFunction, leafIds, RunReporter } from './testModel'; -import { CppUnitReport, parseCppUnitReport, reportedNames, testResults } from './testResults'; +import type { FileDiagnostic, ProblemMatcher } from './problemMatcher'; +import { allIds, type CeedlingFileNode, type CeedlingProjectNode, type CeedlingTestFunction, leafIds, type RunReporter } from './testModel'; +import { type CppUnitReport, parseCppUnitReport, reportedNames, testResults } from './testResults'; import { discoverTests } from './unityTestDiscovery'; export { CeedlingLoadError } from './ceedlingProjectConfig'; @@ -381,8 +381,8 @@ export class CeedlingEngine { const result = await this.ceedling(['version'], { cwd: project.absPath }); const reported = parseCeedlingVersionString(result.stdout); const version = reported === undefined ? null : semver.coerce(reported); - this.logger.debug(`checkCeedlingVersion(${project.key}): ${reported}`); - if (version === null) { + this.logger.debug(`checkCeedlingVersion(${project.key}): ${reported ?? 'none'}`); + if (reported === undefined || version === null) { throw new CeedlingLoadError( `Could not run 'ceedling version' for project '${project.key}'. Check that Ceedling is installed and runs ` + `in the configured shell (ceedlingExplorer.shellPath).\n${result.stdout}\n${result.stderr}` @@ -394,13 +394,13 @@ export class CeedlingEngine { `${MINIMUM_CEEDLING_VERSION} or higher. Please upgrade your Ceedling installation.` ); } - return reported!; + return reported; } // The project's fully merged configuration, from Ceedling itself: project file, mixins, and // plugin defaults, as a real build sees them. Ceedling 1.2.0 can write it to stdout. Earlier // versions write only to a file, so this goes through a uniquely named temporary file. - private async resolveConfig(project: CeedlingProject, capabilities: CeedlingCapabilities): Promise { + private async resolveConfig(project: CeedlingProject, capabilities: CeedlingCapabilities): Promise { if (capabilities.dumpconfigStdout) { const result = await this.ceedling(['dumpconfig', '--stdout'], { project }); if (result.error) { @@ -461,7 +461,9 @@ export class CeedlingEngine { const options = { testPrefix: project.state.testPrefix, useParamTests: project.state.useParamTests }; const testLabel = pretty.tests ? this.labelRegex(() => buildTestLabelRegex(project.state.testPrefix)) : undefined; const fileLabel = pretty.files ? this.labelRegex(() => buildFileLabelRegex(project.state.testFilePrefix)) : undefined; - const label = (regex: RegExp | undefined, text: string) => (regex && regex.exec(text)?.[1]) || text; + // `||`, 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) { const absPath = this.paths.resolve(project.absPath, file); @@ -470,7 +472,7 @@ export class CeedlingEngine { discovered = discoverTests(source ?? await this.fs.readFile(absPath), 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}'): ${e}`); + this.logger.warn(`testFileNodes: skipping '${absPath}' (test prefix '${options.testPrefix}'): ${util.format(e)}`); continue; } for (const warning of discovered.warnings) { @@ -497,7 +499,7 @@ export class CeedlingEngine { try { return build(); } catch (e) { - this.logger.warn(`Pretty labels are unavailable: ${e}`); + this.logger.warn(`Pretty labels are unavailable: ${util.format(e)}`); return undefined; } } @@ -557,7 +559,8 @@ export class CeedlingEngine { const diagnostics = this.problemMatcher.match( result.stdout, result.stderr, project.absPath, config.get('problemMatching.mode', 'disabled'), - config.get('problemMatching.patterns', []), + // Untyped user JSON. ProblemMatcher validates each pattern. + config.get('problemMatching.patterns', []), ); if (scope.kind === 'project') { this.problemMatcher.retainIds((id) => !id.startsWith(`${project.key}::`)); @@ -608,7 +611,7 @@ export class CeedlingEngine { // here, so each trailing part of the file's directory is tried, and the newest build wins. private executableSubpath(project: LoadedProject, file: CeedlingFileNode): string { const segments = file.id.split(/[\\/]/); - const name = segments.pop()!.replace(/\.[^.]*$/, ''); + 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 @@ -656,6 +659,7 @@ export class CeedlingEngine { 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; } diff --git a/src/ceedlingOutputParsing.ts b/src/ceedlingOutputParsing.ts index f9a647c..417097e 100644 --- a/src/ceedlingOutputParsing.ts +++ b/src/ceedlingOutputParsing.ts @@ -9,14 +9,14 @@ export function parseFileListBullets(stdout: string): string[] { return stdout.replace(/\r\n?/g, '\n').split('\n') .map((line) => /^\s*[-•*]\s+(.*)$/.exec(line)) .filter((match): match is RegExpExecArray => match !== null) - .map((match) => match[1].trim()); + .map((match) => (match[1] ?? '').trim()); } // The version in `ceedling version` stdout, e.g. "1.0.0" or a build's "1.1.0-5bbbc8f". undefined // when there is none. The caller decides what that means. export function parseCeedlingVersionString(stdout: string): string | undefined { const match = /^\s*Ceedling\s*(?:::|=>)\s*(.*)$/m.exec(stdout); - return match ? match[1].trim() : undefined; + return match?.[1]?.trim(); } // Captures a test file's pretty label: its name after the configured prefix, without directory or @@ -34,7 +34,7 @@ export function buildTestLabelRegex(testPrefix: string): RegExp { // The function name within a reported test name, without its file or parametrized arguments: // "test/foo.c::test_add(2, 3)" gives "test_add". export function extractTestFunctionName(reportedName: string): string { - const afterFile = reportedName.includes('::') ? reportedName.split('::').pop()! : reportedName; + const afterFile = reportedName.split('::').pop() ?? reportedName; const parenIndex = afterFile.indexOf('('); return parenIndex === -1 ? afterFile : afterFile.slice(0, parenIndex); } @@ -52,7 +52,7 @@ export function testCaseFilterMatchedExactly(requestedFunctionName: string, repo // whole. That version embeds the gdb text in the message instead of writing a log. export function extractGdbLogReference(message: string): { text: string, logPath: string | undefined } { const match = message.match(/\n?\(([^()\n]+\.gdb\.log)\)\s*$/); - if (!match || match.index === undefined) { + if (match?.index === undefined) { return { text: message, logPath: undefined }; } return { text: message.slice(0, match.index), logPath: match[1] }; diff --git a/src/ceedlingProcess.ts b/src/ceedlingProcess.ts index c82fd79..9a0bb0c 100644 --- a/src/ceedlingProcess.ts +++ b/src/ceedlingProcess.ts @@ -75,12 +75,11 @@ export class CeedlingRunner { return Promise.resolve({ error: null, stdout: '', stderr: '', cancelled: true }); } return new Promise((resolve) => { - // Declared ahead of exec: a test double may call back before exec returns. - let child: child_process.ChildProcess | undefined; - let subscription: { dispose(): void } | undefined; - let finished = false; - let cancelled = false; - child = this.exec( + // Shared with the exit callback, which a test double may call before exec even returns. + // The child is tracked as live only if it is still running once exec returns. + const state: { tracked?: child_process.ChildProcess, subscription?: { dispose(): void }, finished: boolean, cancelled: boolean } = + { finished: false, cancelled: false }; + const child = this.exec( buildCommandLine(invocation.command, invocation.args), { cwd: invocation.cwd, @@ -89,22 +88,23 @@ export class CeedlingRunner { maxBuffer: MAX_OUTPUT_BYTES, }, (error, stdout, stderr) => { - finished = true; - if (child) this.live.delete(child); - subscription?.dispose(); + state.finished = true; + if (state.tracked) this.live.delete(state.tracked); + state.subscription?.dispose(); resolve({ error, stdout: invocation.stripAnsi ? stripAnsi(stdout) : stdout, stderr: invocation.stripAnsi ? stripAnsi(stderr) : stderr, - cancelled, + cancelled: state.cancelled, }); }, ); - if (!finished) { + if (!state.finished) { + state.tracked = child; this.live.add(child); - subscription = cancellation?.onCancellationRequested(() => { - cancelled = true; - this.stop(child!); + state.subscription = cancellation?.onCancellationRequested(() => { + state.cancelled = true; + this.stop(child); }); } }); @@ -159,8 +159,7 @@ export function descendantsOf(root: number, parents: ReadonlyMap } const found: number[] = []; const pending = [root]; - while (pending.length > 0) { - const pid = pending.pop()!; + for (let pid = pending.pop(); pid !== undefined; pid = pending.pop()) { found.push(pid); pending.push(...(children.get(pid) ?? [])); } @@ -171,15 +170,19 @@ export function descendantsOf(root: number, parents: ReadonlyMap // itself contain spaces and parentheses, so fields are read after its last closing parenthesis. export function parseProcStatParent(stat: string): number | undefined { const fields = stat.slice(stat.lastIndexOf(')') + 1).trim().split(/\s+/); - const parent = parseInt(fields[1], 10); + const parent = parseInt(fields[1] ?? '', 10); return stat.includes(')') && Number.isFinite(parent) ? parent : undefined; } function linuxProcessParents(): Map { const parents = new Map(); + // Synchronous on purpose: a consistent snapshot of the tree before any kill. /proc is in memory, + // and this runs only on cancellation. + // eslint-disable-next-line no-restricted-properties -- a consistent snapshot, see above for (const entry of fs.readdirSync('/proc')) { if (!/^\d+$/.test(entry)) continue; try { + // eslint-disable-next-line no-restricted-properties -- see above const parent = parseProcStatParent(fs.readFileSync(`/proc/${entry}/stat`, 'utf8')); if (parent !== undefined) parents.set(Number(entry), parent); } catch { diff --git a/src/ceedlingProjectConfig.ts b/src/ceedlingProjectConfig.ts index c828ade..ce200d8 100644 --- a/src/ceedlingProjectConfig.ts +++ b/src/ceedlingProjectConfig.ts @@ -113,7 +113,7 @@ function projectFromSetting( let key = setting.path; if (setting.path.endsWith('.yml')) { const segments = setting.path.split(/[\\/]/); - configuredFile = segments.pop()!; + configuredFile = segments.pop() ?? setting.path; directory = segments.join('/') || '.'; if (configuredFile !== DEFAULT_PROJECT_FILE && configuredFile !== ALTERNATE_DEFAULT_PROJECT_FILE) { key = configuredFile.slice(0, -'.yml'.length); @@ -137,7 +137,7 @@ function projectFiles( const hasDefault = exists(paths.join(absPath, DEFAULT_PROJECT_FILE)); const hasAlternate = 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); + const projectFileName = isDefaultName ? configuredFile : (hasAlternate && !hasDefault ? ALTERNATE_DEFAULT_PROJECT_FILE : DEFAULT_PROJECT_FILE); return { projectFileName, ymlFileName: configuredFile ?? projectFileName, diff --git a/src/logger.ts b/src/logger.ts index c5b3f68..3160f5c 100644 --- a/src/logger.ts +++ b/src/logger.ts @@ -1,6 +1,6 @@ import util from 'util'; import vscode from 'vscode'; -import { LogLevel, LogOutputChannel } from 'vscode'; +import { LogLevel, type LogOutputChannel } from 'vscode'; // Writes to the "Ceedling Explorer" log output channel, created on first use. Its level follows // VS Code's own log level setting for the channel. diff --git a/src/main.ts b/src/main.ts index f7a2cf6..d10f971 100644 --- a/src/main.ts +++ b/src/main.ts @@ -46,7 +46,7 @@ function debugTestExecutableGuard(): never { throw new Error("Start debugging from a test’s debug icon in the Testing view, not with F5 or via the Run and Debug panel."); } -export async function activate(context: vscode.ExtensionContext) { +export function activate(context: vscode.ExtensionContext) { isDevelopmentMode = context.extensionMode === vscode.ExtensionMode.Development; extensionPath = context.extensionPath; const workspaceFolderCount = vscode.workspace.workspaceFolders?.length ?? 0; diff --git a/src/problemMatcher.ts b/src/problemMatcher.ts index b37d7e0..2ff3bf6 100644 --- a/src/problemMatcher.ts +++ b/src/problemMatcher.ts @@ -1,6 +1,6 @@ import { Diagnostic, - DiagnosticCollection, + type DiagnosticCollection, DiagnosticSeverity, Position, Range, @@ -10,13 +10,12 @@ import { import { getPatternProblems, getPatternsPreset, - MatchedProblem, + type MatchedProblem, normalizePatterns, - ProblemMatchingPattern, + type ProblemMatchingPattern, } from './problemMatching'; -import { Logger } from './logger'; - -export { ProblemMatchingPattern } from './problemMatching'; +import util from 'util'; +import { type Logger } from './logger'; // A Diagnostic with the absolute path of the file it belongs to. export interface FileDiagnostic { @@ -52,7 +51,7 @@ export class ProblemMatcher { } // Every problem the configured patterns find in a run's output. No state changes. - match(stdout: string, stderr: string, projectPath: string, mode: string, patterns: ProblemMatchingPattern[]): FileDiagnostic[] { + match(stdout: string, stderr: string, projectPath: string, mode: string, patterns: readonly unknown[]): FileDiagnostic[] { let normalizedPatterns: ProblemMatchingPattern[]; if (mode === "patterns") { const { valid, invalid } = normalizePatterns(patterns); @@ -69,7 +68,7 @@ export class ProblemMatcher { try { fileDiagnostics.push(...getPatternProblems(stdout, stderr, projectPath, pattern).map(toFileDiagnostic)); } catch (e) { - this.logger.warn(`match: pattern threw, likely an invalid regexp (${pattern.regexp}): ${e}`); + this.logger.warn(`match: pattern threw, likely an invalid regexp (${pattern.regexp}): ${util.format(e)}`); } } this.logger.debug(`match: mode=${mode}, patterns=${normalizedPatterns.length}, diagnostics=${fileDiagnostics.length}`); diff --git a/src/problemMatching.ts b/src/problemMatching.ts index 76acede..d340aa9 100644 --- a/src/problemMatching.ts +++ b/src/problemMatching.ts @@ -30,34 +30,41 @@ export interface MatchedProblem { endColumn: number; } -// Sanitizes patterns from settings. WorkspaceConfiguration.get doesn't enforce its type, and -// user JSON can hold anything. `valid` holds each well-shaped pattern, with defaults filled in as +// Sanitizes patterns from settings, which arrive as untyped user JSON: WorkspaceConfiguration +// doesn't enforce its type parameter. `valid` holds each well-shaped pattern, with defaults filled in as // the "gcc" preset's are. `invalid` holds the rest, for the caller to log. -export function normalizePatterns(patterns: ProblemMatchingPattern[]): { valid: ProblemMatchingPattern[], invalid: unknown[] } { +export function normalizePatterns(patterns: readonly unknown[]): { valid: ProblemMatchingPattern[], invalid: unknown[] } { const valid: ProblemMatchingPattern[] = []; const invalid: unknown[] = []; - for (const pattern of patterns) { - const wellShaped = (pattern.regexp !== undefined) && (typeof pattern.regexp === 'string') && - (pattern.message !== undefined) && (typeof pattern.message === 'number') && - (pattern.file !== undefined) && (typeof pattern.file === 'number'); - if (!wellShaped) { - invalid.push(pattern); + for (const candidate of patterns) { + const get = (key: string): unknown => + typeof candidate === 'object' && candidate !== null ? (candidate as Record)[key] : undefined; + const regexp = get('regexp'); + const message = get('message'); + const file = get('file'); + if (typeof regexp !== 'string' || typeof message !== 'number' || typeof file !== 'number') { + invalid.push(candidate); continue; } + const severity = get('severity'); + const filePrefix = get('filePrefix'); + const index = (key: string) => { + const value = get(key); + return typeof value === 'number' ? value : null; + }; valid.push({ - scanStdout: pattern.scanStdout === true, - scanStderr: pattern.scanStderr !== false, - severity: (pattern.severity === 'error' || pattern.severity === 'warning' || pattern.severity === 'info') - ? pattern.severity : 'info', - filePrefix: (pattern.filePrefix !== undefined && typeof pattern.filePrefix === 'string') ? pattern.filePrefix : '', - regexp: pattern.regexp, - message: pattern.message, - file: pattern.file, - line: (pattern.line !== undefined && typeof pattern.line === 'number') ? pattern.line : null, - lastLine: (pattern.lastLine !== undefined && typeof pattern.lastLine === 'number') ? pattern.lastLine : null, - column: (pattern.column !== undefined && typeof pattern.column === 'number') ? pattern.column : null, - lastColumn: (pattern.lastColumn !== undefined && typeof pattern.lastColumn === 'number') ? pattern.lastColumn : null, + scanStdout: get('scanStdout') === true, + scanStderr: get('scanStderr') !== false, + severity: severity === 'error' || severity === 'warning' || severity === 'info' ? severity : 'info', + filePrefix: typeof filePrefix === 'string' ? filePrefix : '', + regexp, + message, + file, + line: index('line'), + lastLine: index('lastLine'), + column: index('column'), + lastColumn: index('lastColumn'), }); } @@ -145,16 +152,20 @@ export function matchToProblem( return undefined; } - const fileValue = resolveDiagnosticFilePath(matches[file], filePrefix); + // An optional group that didn't participate in the match is undefined despite a valid index. + const matchedFile = matches[file]; const messageValue = matches[message]; + if (matchedFile === undefined || messageValue === undefined) { + return undefined; + } + const fileValue = resolveDiagnosticFilePath(matchedFile, filePrefix); const lineValue = line !== null ? Number(matches[line]) : undefined; const lastLineValue = lastLine !== null ? Number(matches[lastLine]) : undefined; const columnValue = column !== null ? Number(matches[column]) : undefined; const lastColumnValue = lastColumn !== null ? Number(matches[lastColumn]) : undefined; const isNumberOrUndefined = (value: number | undefined) => value === undefined || !Number.isNaN(value); - if (fileValue === undefined || messageValue === undefined || - !isNumberOrUndefined(lineValue) || !isNumberOrUndefined(lastLineValue) || + if (!isNumberOrUndefined(lineValue) || !isNumberOrUndefined(lastLineValue) || !isNumberOrUndefined(columnValue) || !isNumberOrUndefined(lastColumnValue)) { return undefined; } diff --git a/src/runPlan.ts b/src/runPlan.ts index a535122..0e5c164 100644 --- a/src/runPlan.ts +++ b/src/runPlan.ts @@ -4,7 +4,7 @@ // (`test:all`), a whole file, or one function in a file. It has no `vscode` dependency. The // controller translates TestItems into PlanTargets. -import { CeedlingFileNode, CeedlingProjectNode, CeedlingTestFunction } from './testModel'; +import { type CeedlingFileNode, type CeedlingProjectNode, type CeedlingTestFunction } from './testModel'; export type PlanTarget = | { kind: 'project', projectKey: string } @@ -32,7 +32,7 @@ export function buildRunPlan( const tree = new Map(projects.map((project) => [project.projectKey, project])); const findFile = (projectKey: string, fileId: string) => tree.get(projectKey)?.files.find((f) => f.id === fileId); const selection = new Map(); - const selectProject = (projectKey: string) => getOrAdd(selection, projectKey, () => ({ whole: false, files: new Map() })); + const selectProject = (projectKey: string) => getOrAdd(selection, projectKey, () => ({ whole: false, files: new Map() })); const selectFile = (projectKey: string, fileId: string) => getOrAdd(selectProject(projectKey).files, fileId, () => ({ whole: false, functions: new Set() })); @@ -87,7 +87,7 @@ export function buildRunPlan( } for (const { projectKey, fileId, functionId, caseIds } of excludedCases.values()) { const fn = findFile(projectKey, fileId)?.functions.find((f) => f.id === functionId); - if (fn && fn.cases.every((c) => caseIds.has(c.id))) { + if (fn?.cases.every((c) => caseIds.has(c.id))) { excludeFunction(projectKey, fileId, functionId); } } diff --git a/src/singleFlight.ts b/src/singleFlight.ts index 0d49227..f735c5a 100644 --- a/src/singleFlight.ts +++ b/src/singleFlight.ts @@ -3,6 +3,8 @@ // it, however many requests arrive. Each caller's promise settles when a run that started after // its request finishes. Startup, file watchers, and settings changes can all ask for discovery at // once, and each discovery starts several Ceedling processes. +type Outcome = { failed: false } | { failed: true, reason: unknown }; + export class SingleFlight { private flight: Promise | undefined; private rerun = false; @@ -21,21 +23,28 @@ export class SingleFlight { // Repeats the work while requests keep arriving, then rejects with the last run's error, if any. private async fly(): Promise { try { - let error: unknown; + let outcome: Outcome; do { this.rerun = false; - error = undefined; - try { - await this.work(); - } catch (e) { - error = e; - } + // eslint-disable-next-line no-await-in-loop -- each rerun must follow the run before it + outcome = await this.attempt(); + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- run() sets it during the await } while (this.rerun); - if (error !== undefined) { - throw error; + if (outcome.failed) { + throw outcome.reason; } } finally { this.flight = undefined; } } + + // A failure is recorded as a flag, so a rejection whose reason is itself undefined still counts. + private async attempt(): Promise { + try { + await this.work(); + return { failed: false }; + } catch (reason) { + return { failed: true, reason }; + } + } } diff --git a/src/testController.ts b/src/testController.ts index abcbe57..8c6936b 100644 --- a/src/testController.ts +++ b/src/testController.ts @@ -4,11 +4,11 @@ import util from 'util'; import vscode from 'vscode'; import { CeedlingEngine, CeedlingLoadError } from './ceedlingEngine'; import { extractGdbLogReference } from './ceedlingOutputParsing'; -import { Logger } from './logger'; +import { type Logger } from './logger'; import { ProblemMatcher } from './problemMatcher'; -import { buildRunPlan, PlanTarget, RunPlanEntry } from './runPlan'; +import { buildRunPlan, type PlanTarget, type RunPlanEntry } from './runPlan'; import { SingleFlight } from './singleFlight'; -import { CeedlingFileNode, CeedlingProjectNode, CeedlingTestFunction, RunReporter } from './testModel'; +import { type CeedlingFileNode, type CeedlingProjectNode, type CeedlingTestFunction, type RunReporter } from './testModel'; // Names the launch configuration of one debug request, so its debug session can be told apart // from any other session VS Code starts meanwhile. @@ -59,12 +59,12 @@ export class CeedlingTestController { vscode.workspace.onDidChangeConfiguration((event) => this.onConfigurationChanged(event)), vscode.workspace.onDidChangeTextDocument((event) => this.scheduleReparse(event.document)), // Closing a file with unsaved edits discards them, so the saved file is what remains. - vscode.workspace.onDidCloseTextDocument((document) => this.reparse(document.uri.fsPath)), + vscode.workspace.onDidCloseTextDocument((document) => void this.reparse(document.uri.fsPath)), ); // Discover now rather than when VS Code first calls resolveHandler, so gutter icons appear // in open test files before the Testing view is ever opened. - this.refresh(); + void this.refresh(); } setLabel(label: string): void { @@ -85,7 +85,7 @@ export class CeedlingTestController { dispose(): void { this.logger.debug(`CeedlingTestController disposed for ${this.workspaceFolder.uri.toString()}`); this.pendingReparses.forEach((timer) => clearTimeout(timer)); - [...this.configurationWatchers.values(), ...this.sourceWatchers.values(), ...this.disposables].forEach((d) => d.dispose()); + [...this.configurationWatchers.values(), ...this.sourceWatchers.values(), ...this.disposables].forEach((d) => { d.dispose(); }); this.disposeController(); this.engine.dispose(); } @@ -112,7 +112,7 @@ export class CeedlingTestController { } private disposeController(): void { - this.controllerDisposables.forEach((d) => d.dispose()); + this.controllerDisposables.forEach((d) => { d.dispose(); }); this.controllerDisposables = []; this.controller = undefined; this.items.clear(); @@ -127,7 +127,7 @@ export class CeedlingTestController { const changed = DISCOVERY_SETTINGS.filter(affects); if (changed.length > 0) { this.logger.debug(`onConfigurationChanged: ${changed.join(', ')} changed, refreshing`); - this.refresh(); + void this.refresh(); } } @@ -179,19 +179,19 @@ export class CeedlingTestController { (watcher, file) => { const changed = () => { this.logger.debug(`watcher: '${file}' changed, refreshing`); - this.refresh(); + void this.refresh(); }; return [watcher.onDidCreate(changed), watcher.onDidChange(changed), watcher.onDidDelete(changed)]; }); this.syncWatchers(this.sourceWatchers, projectDirectories, (directory) => new vscode.RelativePattern(vscode.Uri.file(directory), '**/*.c'), (watcher) => [ - watcher.onDidChange((uri) => this.reparse(uri.fsPath)), + watcher.onDidChange((uri) => void this.reparse(uri.fsPath)), watcher.onDidCreate((uri) => { - if (this.engine.mayBeTestFile(uri.fsPath)) this.refresh(); + if (this.engine.mayBeTestFile(uri.fsPath)) void this.refresh(); }), watcher.onDidDelete((uri) => { - if (this.engine.testFileAt(uri.fsPath)) this.refresh(); + if (this.engine.testFileAt(uri.fsPath)) void this.refresh(); }), ]); } @@ -224,17 +224,27 @@ export class CeedlingTestController { clearTimeout(this.pendingReparses.get(file)); this.pendingReparses.set(file, setTimeout(() => { this.pendingReparses.delete(file); - this.reparse(file, document.getText()); + void this.reparse(file, document.getText()); }, EDIT_REPARSE_DELAY_MS)); } - // Replaces one test file's functions and cases in the tree, without running Ceedling. + // Replaces one test file's functions and cases in the tree, without running Ceedling. Watchers + // and editor events start it without awaiting, so it reports its own failures. private async reparse(absPath: string, source?: string): Promise { + try { + await this.replaceFileItems(absPath, source); + } catch (e) { + this.logger.error(`reparse: '${absPath}' failed: ${util.format(e)}`); + } + } + + private async replaceFileItems(absPath: string, source?: string): Promise { const found = this.engine.testFileAt(absPath); const file = found && await this.engine.rediscoverFile(found.projectKey, found.fileId, source); const projectItems = found && this.items.get(found.projectKey); const fileItem = file && projectItems?.get(file.id); - if (!found || !file || !projectItems || !fileItem) { + const controller = this.controller; + if (!found || !file || !projectItems || !fileItem || !controller) { return; } const project = this.projects.find((p) => p.projectKey === found.projectKey); @@ -246,7 +256,7 @@ export class CeedlingTestController { } const uri = vscode.Uri.file(file.absPath); fileItem.label = file.label; - fileItem.children.replace(file.functions.map((fn) => this.buildFunctionItem(found.projectKey, file, fn, uri, projectItems))); + fileItem.children.replace(file.functions.map((fn) => this.buildFunctionItem(controller, found.projectKey, file, fn, uri, projectItems))); this.logger.trace(`reparse: '${absPath}' -> ${file.functions.length} function(s)`); } @@ -261,7 +271,7 @@ export class CeedlingTestController { for (const project of this.projects) { const projectItems = new Map(); this.items.set(project.projectKey, projectItems); - const fileItems = project.files.map((file) => this.buildFileItem(project.projectKey, file, projectItems)); + const fileItems = project.files.map((file) => this.buildFileItem(controller, project.projectKey, file, projectItems)); if (multiProject) { const projectItem = controller.createTestItem(project.projectKey, project.label); projectItem.children.replace(fileItems); @@ -275,24 +285,27 @@ export class CeedlingTestController { this.logger.debug(`rebuildTree: ${this.projects.length} project(s)`); } - private buildFileItem(projectKey: string, file: CeedlingFileNode, projectItems: Map): vscode.TestItem { + private buildFileItem( + controller: vscode.TestController, projectKey: string, file: CeedlingFileNode, projectItems: Map, + ): vscode.TestItem { const uri = vscode.Uri.file(file.absPath); - const fileItem = this.controller!.createTestItem(file.id, file.label, uri); + const fileItem = controller.createTestItem(file.id, file.label, uri); this.targets.set(fileItem, { kind: 'file', projectKey, fileId: file.id }); projectItems.set(file.id, fileItem); - fileItem.children.replace(file.functions.map((fn) => this.buildFunctionItem(projectKey, file, fn, uri, projectItems))); + fileItem.children.replace(file.functions.map((fn) => this.buildFunctionItem(controller, projectKey, file, fn, uri, projectItems))); return fileItem; } private buildFunctionItem( - projectKey: string, file: CeedlingFileNode, fn: CeedlingTestFunction, uri: vscode.Uri, projectItems: Map, + controller: vscode.TestController, projectKey: string, file: CeedlingFileNode, fn: CeedlingTestFunction, uri: vscode.Uri, + projectItems: Map, ): vscode.TestItem { - const item = this.controller!.createTestItem(fn.id, fn.label, uri); + const item = controller.createTestItem(fn.id, fn.label, uri); item.range = new vscode.Range(fn.line, 0, fn.line, 0); this.targets.set(item, { kind: 'function', projectKey, fileId: file.id, functionId: fn.id }); projectItems.set(fn.id, item); item.children.replace(fn.cases.map((testCase) => { - const caseItem = this.controller!.createTestItem(testCase.id, testCase.label, uri); + const caseItem = controller.createTestItem(testCase.id, testCase.label, uri); caseItem.range = new vscode.Range(testCase.line, 0, testCase.line, 0); this.targets.set(caseItem, { kind: 'case', projectKey, fileId: file.id, functionId: fn.id, caseId: testCase.id }); projectItems.set(testCase.id, caseItem); @@ -402,6 +415,8 @@ export class CeedlingTestController { for (const item of plan.flatMap((entry) => this.itemsOf(entry))) { if (!excluded.has(item)) run.enqueued(item); } + // Entries run one at a time, in tree order. The engine serializes Ceedling runs anyway. + /* eslint-disable no-await-in-loop -- sequential by design, see above */ for (const entry of plan) { if (token.isCancellationRequested) break; const reporter = this.makeReporter(run, entry.projectKey, excluded); @@ -413,6 +428,7 @@ export class CeedlingTestController { await this.engine.runFile(entry.projectKey, entry.file, reporter, token); } } + /* eslint-enable no-await-in-loop */ } catch (e) { this.logger.error(`handleRun error: ${util.format(e)}`); } finally { @@ -480,7 +496,7 @@ export class CeedlingTestController { } catch (e) { this.logger.error(`Debug error: ${util.format(e)}`); } finally { - listeners.forEach((listener) => listener.dispose()); + listeners.forEach((listener) => { listener.dispose(); }); run.end(); } } @@ -500,6 +516,6 @@ export class CeedlingTestController { this.logger.debug(`resolveLaunchConfig: resolved '${debugLaunchConfig}' with executableSubpath='${executableSubpath}'`); return JSON.parse( JSON.stringify(found).replace(/\$\{command:ceedlingExplorer\.debugTestExecutable\}/g, () => executableSubpath) - ); + ) as vscode.DebugConfiguration; } } diff --git a/src/testResults.ts b/src/testResults.ts index a21349b..ad46a01 100644 --- a/src/testResults.ts +++ b/src/testResults.ts @@ -26,26 +26,31 @@ export async function parseCppUnitReport(text: string): Promise { // for a category holding one , and an empty string for a self-closing empty category. Both // become lists here. Throws when there is no at all. export function readCppUnitReport(parsedXml: unknown): CppUnitReport { - const testRun = (parsedXml as { TestRun?: Record } | undefined)?.TestRun; + const testRun = field(parsedXml, 'TestRun'); if (typeof testRun !== 'object' || testRun === null) { throw new Error('The CppUnit report has no element.'); } return { - passed: category(testRun, 'SuccessfulTests').map((test) => ({ name: text(test.Name) })), + passed: category(testRun, 'SuccessfulTests').map((test) => ({ name: text(field(test, 'Name')) })), failed: category(testRun, 'FailedTests').map((test) => { - const line = parseInt(text((test.Location as Record | undefined)?.Line), 10); - return { name: text(test.Name), message: text(test.Message), line: Number.isFinite(line) ? line - 1 : undefined }; + const line = parseInt(text(field(field(test, 'Location'), 'Line')), 10); + return { name: text(field(test, 'Name')), message: text(field(test, 'Message')), line: Number.isFinite(line) ? line - 1 : undefined }; }), - ignored: category(testRun, 'IgnoredTests').map((test) => ({ name: text(test.Name) })), + ignored: category(testRun, 'IgnoredTests').map((test) => ({ name: text(field(test, 'Name')) })), }; } -function category(testRun: Record, name: string): Array> { - const tests = (testRun[name] as { Test?: unknown } | undefined)?.Test; +function category(testRun: object, name: string): unknown[] { + const tests = field(field(testRun, name), 'Test'); if (tests === undefined || tests === null) { return []; } - return (Array.isArray(tests) ? tests : [tests]) as Array>; + return Array.isArray(tests) ? tests : [tests]; +} + +// The value at `key` when `value` is an object, else undefined. Parsed XML is untyped. +function field(value: unknown, key: string): unknown { + return typeof value === 'object' && value !== null ? (value as Record)[key] : undefined; } function text(value: unknown): string { diff --git a/src/unityTestDiscovery.ts b/src/unityTestDiscovery.ts index 7b55e48..e0a7214 100644 --- a/src/unityTestDiscovery.ts +++ b/src/unityTestDiscovery.ts @@ -22,6 +22,11 @@ type MarkerKind = 'CASE' | 'RANGE' | 'MATRIX'; type Marker = { kind: MarkerKind, args: string[] }; type FoundTest = { name: string, markers: Marker[] }; +// A capture group that the pattern's structure guarantees participates in every match. +function captured(match: RegExpMatchArray, group: number): string { + return match[group] ?? ''; +} + export function discoverTests(source: string, options: DiscoveryOptions): DiscoveryResult { const warnings: string[] = []; const found = findTests(source, options, warnings); @@ -40,11 +45,14 @@ const UNHIDE: Record = { '@co@': '{', '@cc@': '}', '@ss@': ';', // preprocessor lines and at `;`, `{`, and `}`. A test signature must then sit within one piece. function logicalLines(source: string): string[] { let text = source.replace(/\\"/g, '@quote@').replace(/\\'/g, '@apos@'); - text = text.replace(/("[^"\n]*")|('[^'\n]*')/g, (literal) => literal.replace(/[{};/]/g, (c) => HIDE_IN_STRINGS[c])); + text = text.replace(/("[^"\n]*")|('[^'\n]*')/g, (literal) => literal.replace(/[{};/]/g, (c) => HIDE_IN_STRINGS[c] ?? c)); text = stripComments(text, () => ''); + // Splitting on capture groups yields undefined for each group that didn't participate, though + // the library types the result as string[]. return text.split(/(^\s*#.*$)|(;|\{|\})/m) + // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition -- see above .filter((piece): piece is string => piece !== undefined) - .map((piece) => piece.replace(/@co@|@cc@|@ss@|@fs@|@quote@|@apos@/g, (token) => UNHIDE[token])); + .map((piece) => piece.replace(/@co@|@cc@|@ss@|@fs@|@quote@|@apos@/g, (token) => UNHIDE[token] ?? token)); } // Unity's three comment passes, in its order. The first removes a line comment that hides the @@ -70,7 +78,8 @@ function findTests(source: string, options: DiscoveryOptions, warnings: string[] if (!match) { continue; } - const [, markerBlock, name] = match; + const markerBlock = captured(match, 1); + const name = captured(match, 2); // Unity keeps only the first occurrence of a name, e.g. a prototype before its definition. if (seen.has(name)) { continue; @@ -89,7 +98,7 @@ function parseMarkers(name: string, markerBlock: string, warnings: string[]): Ma const markers: Marker[] = []; for (let i = 1; i < pieces.length; i += 2) { const kind = pieces[i] as MarkerKind; - const body = pieces[i + 1]; + const body = pieces[i + 1] ?? ''; if (kind === 'CASE') { // Unity trims inside the parentheses, so `TEST_CASE( 1, 2 )` reports as `(1, 2)`. markers.push({ kind, args: [body.replace(/^\s*\(\s*(.*?)\s*\)\s*$/ms, '$1')] }); @@ -108,8 +117,9 @@ const RANGE_PATTERN = /(\[|<)\s*(-?\d+.?\d*)\s*,\s*(-?\d+.?\d*)\s*,\s*(-?\d+.?\d function expandRangeMarker(name: string, body: string, warnings: string[]): string[] { const ranges: string[][] = []; - for (const [, open, start, end, step, close] of body.matchAll(RANGE_PATTERN)) { - const values = expandRubyRange(start, end, step, open === '<' && close === '>'); + for (const match of body.matchAll(RANGE_PATTERN)) { + const [start, end, step] = [captured(match, 2), captured(match, 3), captured(match, 4)] as const; + const values = expandRubyRange(start, end, step, captured(match, 1) === '<' && captured(match, 5) === '>'); if (values === undefined) { warnings.push(`TEST_RANGE for ${name} has a zero step (${start}, ${end}, ${step}); Ceedling cannot build this test.`); return []; @@ -121,6 +131,7 @@ function expandRangeMarker(name: string, body: string, warnings: string[]): stri // Unity's element pattern: a double-quoted string, a character literal, or a bare token (which // may contain a bracketed index). Kept as Unity's own source text so the two compare directly. +// eslint-disable-next-line no-useless-escape -- Unity's source escapes `[` inside the class const MATRIX_ELEMENT = /(?:(?:"(?:\\"|[^\\])*?")+|(?:'\\?.')+|(?:[^\s\]\["',]|\[[\d\S_-]+\])+)/.source; const MATRIX_LIST_PATTERN = new RegExp(`\\[((?:\\s*${MATRIX_ELEMENT}\\s*,?)*(?:\\s*${MATRIX_ELEMENT})?\\s*)\\]`, 'gs'); const MATRIX_ELEMENT_PATTERN = new RegExp(`\\s*(${MATRIX_ELEMENT})\\s*,\\s*`, 'gs'); @@ -128,7 +139,7 @@ const MATRIX_ELEMENT_PATTERN = new RegExp(`\\s*(${MATRIX_ELEMENT})\\s*,\\s*`, 'g function expandMatrixMarker(name: string, body: string, warnings: string[]): string[] { // A trailing comma lets every element, the last included, match the comma-terminated pattern. const lists = [...body.matchAll(MATRIX_LIST_PATTERN)] - .map(([, list]) => [...`${list},`.matchAll(MATRIX_ELEMENT_PATTERN)].map(([, element]) => element)); + .map((list) => [...`${captured(list, 1)},`.matchAll(MATRIX_ELEMENT_PATTERN)].map((element) => captured(element, 1))); if (lists.length === 0) { warnings.push(`TEST_MATRIX for ${name} has no [ ... ] lists; Ceedling cannot build this test.`); return []; @@ -155,11 +166,12 @@ function cartesianProduct(lists: string[][]): string[] { // as floats. A negative step counts down, as Ruby's does. export function expandRubyRange(start: string, end: string, step: string, excludeEnd: boolean): string[] | undefined { const isFloat = [start, end, step].some((value) => value.includes('.')); + const parse = (value: string) => isFloat ? parseFloat(value) : parseInt(value, 10); + const [b, e, s] = [parse(start), parse(end), parse(step)] as const; + if (s === 0) { + return undefined; + } if (!isFloat) { - const [b, e, s] = [start, end, step].map((value) => parseInt(value, 10)); - if (s === 0) { - return undefined; - } const values: string[] = []; const inRange = (v: number) => s > 0 ? (excludeEnd ? v < e : v <= e) : (excludeEnd ? v > e : v >= e); for (let v = b; inRange(v); v += s) { @@ -167,10 +179,6 @@ export function expandRubyRange(start: string, end: string, step: string, exclud } return values; } - const [b, e, s] = [start, end, step].map((value) => parseFloat(value)); - if (s === 0) { - return undefined; - } const count = rubyFloatStepCount(b, e, s, excludeEnd); const values: string[] = []; for (let i = 0; i < count; i++) { @@ -214,7 +222,8 @@ export function rubyFloatToString(value: number): string { if (Object.is(value, -0)) { return '-0.0'; } - const [mantissa, exponentText] = value.toExponential().split('e'); + // toExponential() always yields `d[.ddd]e±N`, so both parts exist. + const [mantissa = '', exponentText = '0'] = value.toExponential().split('e'); const exponent = parseInt(exponentText, 10); if (exponent >= -4 && exponent <= 14) { const fixed = String(value); @@ -239,11 +248,16 @@ function blankCommentsAndStrings(source: string): string { function locateTests(source: string, found: FoundTest[]): DiscoveredTest[] { const blanked = blankCommentsAndStrings(source); - const lineStarts = [0, ...[...blanked.matchAll(/\n/g)].map((m) => m.index! + 1)]; + const lineStarts = [0, ...[...blanked.matchAll(/\n/g)].map((m) => m.index + 1)]; + // The last line starting at or before `offset`, by binary search over the line starts. const lineAt = (offset: number) => { - let line = 0; - while (line + 1 < lineStarts.length && lineStarts[line + 1] <= offset) line++; - return line; + let low = 0; + let high = lineStarts.length - 1; + while (low < high) { + const middle = Math.ceil((low + high) / 2); + if ((lineStarts[middle] ?? Infinity) <= offset) low = middle; else high = middle - 1; + } + return low; }; // Tests are found in source order, so each search starts where the previous one ended. @@ -276,7 +290,7 @@ function locateMarkers(between: string, offset: number, count: number): number[] } const boundaries = [...between.matchAll(/[;{}]|^[ \t]*#.*$/gm)]; const last = boundaries[boundaries.length - 1]; - const start = last ? last.index! + last[0].length : 0; - const markers = [...between.slice(start).matchAll(/TEST_(?:CASE|RANGE|MATRIX)\s*\(/g)].map((m) => offset + start + m.index!); + const start = last ? last.index + last[0].length : 0; + const markers = [...between.slice(start).matchAll(/TEST_(?:CASE|RANGE|MATRIX)\s*\(/g)].map((m) => offset + start + m.index); return markers.length === count ? markers : []; } diff --git a/tests/integration/main.ts b/tests/integration/main.ts index 1288e03..511849b 100644 --- a/tests/integration/main.ts +++ b/tests/integration/main.ts @@ -20,4 +20,4 @@ async function main() { } } -main(); +void main(); diff --git a/tests/integration/runner.ts b/tests/integration/runner.ts index e179a9f..b75bad5 100644 --- a/tests/integration/runner.ts +++ b/tests/integration/runner.ts @@ -29,7 +29,7 @@ export function run(): Promise { } }); } catch (err) { - e(err); + e(err instanceof Error ? err : new Error(String(err))); } }); }); diff --git a/tests/unit/ceedlingEngine.test.ts b/tests/unit/ceedlingEngine.test.ts index 562671f..8906f27 100644 --- a/tests/unit/ceedlingEngine.test.ts +++ b/tests/unit/ceedlingEngine.test.ts @@ -1,14 +1,14 @@ import * as assert from 'assert'; -import * as child_process from 'child_process'; +import type * as child_process from 'child_process'; import * as fs from 'fs'; import * as path from 'path'; import type vscode from 'vscode'; -import { CeedlingEngine, EngineFileSystem } from '../../src/ceedlingEngine'; -import { Cancellation, CeedlingRunner } from '../../src/ceedlingProcess'; +import { CeedlingEngine, type EngineFileSystem } from '../../src/ceedlingEngine'; +import { type Cancellation, CeedlingRunner } from '../../src/ceedlingProcess'; import { CeedlingLoadError } from '../../src/ceedlingProjectConfig'; import type { Logger } from '../../src/logger'; import type { FileDiagnostic, ProblemMatcher } from '../../src/problemMatcher'; -import { CeedlingFileNode, CeedlingProjectNode, leafIds, RunReporter } from '../../src/testModel'; +import { type CeedlingFileNode, type CeedlingProjectNode, leafIds, type RunReporter } from '../../src/testModel'; // Drives CeedlingEngine against a simulated Ceedling: an in-memory filesystem, and an exec double // that answers each Ceedling command the way the real one does. See FakeCeedling below. Tests @@ -65,7 +65,7 @@ class FakeCeedling { const exec = ((command: string, options: child_process.ExecOptions, callback: any) => { const cwd = String(options.cwd); this.commands.push({ command, cwd }); - Promise.resolve(this.reply(command, cwd)).then((reply) => { + 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 ?? ''); }); diff --git a/tests/unit/ceedlingOutputParsing.test.ts b/tests/unit/ceedlingOutputParsing.test.ts index cdf081d..372f5b2 100644 --- a/tests/unit/ceedlingOutputParsing.test.ts +++ b/tests/unit/ceedlingOutputParsing.test.ts @@ -2,7 +2,6 @@ import * as assert from 'assert'; import * as fs from 'fs'; import * as path from 'path'; import * as semver from 'semver'; -import * as xml2js from 'xml2js'; import { buildFileLabelRegex, buildTestLabelRegex, @@ -12,6 +11,7 @@ import { parseFileListBullets, testCaseFilterMatchedExactly, } from '../../src/ceedlingOutputParsing'; +import { parseCppUnitReport } from '../../src/testResults'; // Fixtures stay in the source tree. Compiled tests run from out/tests/unit, three levels below. const fixturesRoot = path.resolve(__dirname, '..', '..', '..', 'tests', 'fixtures'); @@ -62,7 +62,7 @@ suite('parseCeedlingVersionString', () => { const stdout = readFixture('ceedling-1.1.0', 'version-stdout.txt'); const version = parseCeedlingVersionString(stdout); assert.strictEqual(version, '1.1.0-5bbbc8f'); - assert.strictEqual(semver.lt(version!, '1.0.0'), false); + assert.strictEqual(semver.lt(version, '1.0.0'), false); }); test('returns undefined when no "Ceedling => x.y.z" line is present', () => { @@ -75,7 +75,7 @@ suite('buildFileLabelRegex / buildTestLabelRegex', () => { const regex = buildFileLabelRegex('test_'); const match = regex.exec('test/test_calculator.c'); assert.ok(match); - assert.strictEqual(match![1], 'calculator'); + assert.strictEqual(match[1], 'calculator'); }); test('buildFileLabelRegex labels a file at the project root and one under backslashes', () => { @@ -94,7 +94,7 @@ suite('buildFileLabelRegex / buildTestLabelRegex', () => { const regex = buildTestLabelRegex('test|spec|should'); const match = regex.exec('test_add_should_ReturnSum'); assert.ok(match); - assert.strictEqual(match![1], 'add_should_ReturnSum'); + assert.strictEqual(match[1], 'add_should_ReturnSum'); }); }); @@ -143,18 +143,11 @@ suite('testCaseFilterMatchedExactly', () => { }); suite('extractGdbLogReference', () => { - function parseXml(xmlText: string): Promise { - const parser = new xml2js.Parser({ explicitArray: false }); - return new Promise((resolve, reject) => { - parser.parseString(xmlText, (error: any, result: any) => error ? reject(error) : resolve(result)); - }); - } - // Captured from Ceedling 1.1.0 with :use_backtrace: :gdb, for a test dereferencing null. test('splits a real Ceedling 1.1.0 :use_backtrace: :gdb crash message', async () => { const xmlText = readFixture('ceedling-1.1.0', 'cppunit-report-crash.xml'); - const parsed = await parseXml(xmlText); - const message = parsed.TestRun.FailedTests.Test.Message; + const [crash] = (await parseCppUnitReport(xmlText)).failed; + const message = crash.message; const { text, logPath } = extractGdbLogReference(message); assert.strictEqual(logPath, 'build/logs/test/test_calculator/test_should_crash.gdb.log'); diff --git a/tests/unit/ceedlingProcess.test.ts b/tests/unit/ceedlingProcess.test.ts index 1b1b3bf..2fc7255 100644 --- a/tests/unit/ceedlingProcess.test.ts +++ b/tests/unit/ceedlingProcess.test.ts @@ -1,6 +1,6 @@ import * as assert from 'assert'; -import * as child_process from 'child_process'; -import { buildCommandLine, Cancellation, CeedlingInvocation, CeedlingRunner, ceedlingEnvironment, descendantsOf, parseProcStatParent, quoteIfNeeded } from '../../src/ceedlingProcess'; +import type * as child_process from 'child_process'; +import { buildCommandLine, type Cancellation, type CeedlingInvocation, CeedlingRunner, ceedlingEnvironment, descendantsOf, parseProcStatParent, quoteIfNeeded } from '../../src/ceedlingProcess'; type ExecCall = { command: string, options: child_process.ExecOptions, finish: (error: Error | null, stdout: string, stderr: string) => void }; @@ -133,8 +133,8 @@ suite('CeedlingRunner', () => { }) as unknown as typeof child_process.exec; const runner = new CeedlingRunner({ exec, kill: (p) => killed.push(p) }); const done = runner.run(invocation()); - runner.run(invocation()); - runner.run(invocation()); + void runner.run(invocation()); + void runner.run(invocation()); calls[0].finish(null, '', ''); await done; runner.cancelAll(); diff --git a/tests/unit/problemMatching.test.ts b/tests/unit/problemMatching.test.ts index 6e72f5c..54ffcef 100644 --- a/tests/unit/problemMatching.test.ts +++ b/tests/unit/problemMatching.test.ts @@ -7,7 +7,7 @@ import { getPatternsPreset, matchToProblem, normalizePatterns, - ProblemMatchingPattern, + type ProblemMatchingPattern, resolveDiagnosticFilePath, } from '../../src/problemMatching'; @@ -113,7 +113,7 @@ suite('matchToProblem', () => { function exec(regexp: string, line: string): RegExpExecArray { const matches = new RegExp(regexp).exec(line); assert.ok(matches, `expected '${regexp}' to match '${line}'`); - return matches!; + return matches; } test('builds a 0-based problem from a real error-pattern match', () => { @@ -135,6 +135,21 @@ suite('matchToProblem', () => { assert.strictEqual(matchToProblem(matches, 1, 99, 'error', '', 2, null, null, null), undefined); }); + // An optional group that didn't participate is undefined, though its index is in range. + test('rejects a match whose file group did not participate', () => { + const matches = exec('^(?:(\\S+\\.c):)?(\\d+): (.*)$', '9: whatever'); + assert.strictEqual(matchToProblem(matches, 1, 3, 'error', '/project', 2, null, null, null), undefined); + }); + + test('a pattern with an optional file group still reports the lines where it matched', () => { + const pattern: ProblemMatchingPattern = { + scanStdout: false, scanStderr: true, severity: 'error', filePrefix: '/project', + regexp: '^(?:(\\S+\\.c):)?(\\d+): (.*)$', message: 3, file: 1, line: 2, lastLine: null, column: null, lastColumn: null, + }; + const problems = getPatternProblems('', '9: no file\nsrc/a.c:4: has file', '/project', pattern); + assert.deepStrictEqual(problems.map((p) => [p.file, p.startLine]), [[path.join('/project', 'src/a.c'), 3]]); + }); + test('rejects a match when a captured line value is not a number', () => { const matches = exec('^(.*):(\\w+):(.*)$', 'test/foo.c:NaN:whatever'); assert.strictEqual(matchToProblem(matches, 1, 3, 'error', '', 2, null, null, null), undefined); @@ -144,7 +159,7 @@ suite('matchToProblem', () => { suite('normalizePatterns', () => { test('defaults every optional field, and keeps a well-shaped pattern', () => { const { valid, invalid } = normalizePatterns([ - { regexp: '^(.*)$', message: 0, file: 0 } as unknown as ProblemMatchingPattern, + { regexp: '^(.*)$', message: 0, file: 0 }, ]); assert.deepStrictEqual(invalid, []); assert.deepStrictEqual(valid, [{ @@ -164,7 +179,7 @@ suite('normalizePatterns', () => { test('drops a pattern missing a required field', () => { const { valid, invalid } = normalizePatterns([ - { message: 0, file: 0 } as unknown as ProblemMatchingPattern, // no regexp + { message: 0, file: 0 }, // no regexp ]); assert.deepStrictEqual(valid, []); assert.strictEqual(invalid.length, 1); @@ -172,7 +187,7 @@ suite('normalizePatterns', () => { test('drops a pattern whose required field is the wrong type', () => { const { valid, invalid } = normalizePatterns([ - { regexp: '^(.*)$', message: '0', file: 0 } as unknown as ProblemMatchingPattern, // message is a string + { regexp: '^(.*)$', message: '0', file: 0 }, // message is a string ]); assert.deepStrictEqual(valid, []); assert.strictEqual(invalid.length, 1); diff --git a/tests/unit/runPlan.test.ts b/tests/unit/runPlan.test.ts index f826b68..86720d0 100644 --- a/tests/unit/runPlan.test.ts +++ b/tests/unit/runPlan.test.ts @@ -1,6 +1,6 @@ import * as assert from 'assert'; -import { buildRunPlan, PlanTarget, RunPlanEntry } from '../../src/runPlan'; -import { CeedlingFileNode, CeedlingProjectNode } from '../../src/testModel'; +import { buildRunPlan, type PlanTarget, type RunPlanEntry } from '../../src/runPlan'; +import { type CeedlingFileNode, type CeedlingProjectNode } from '../../src/testModel'; function file(projectKey: string, id: string, functions: Array): CeedlingFileNode { return { diff --git a/tests/unit/singleFlight.test.ts b/tests/unit/singleFlight.test.ts index 1e7f111..676835c 100644 --- a/tests/unit/singleFlight.test.ts +++ b/tests/unit/singleFlight.test.ts @@ -42,15 +42,24 @@ suite('SingleFlight', () => { test('every caller resolves only after the run that covers its request', async () => { const work = gatedWork(); const flight = new SingleFlight(work.run); - flight.run(); + void flight.run(); let lateDone = false; - flight.run().then(() => { lateDone = true; }); + void flight.run().then(() => { lateDone = true; }); await work.releaseNext(); assert.strictEqual(lateDone, false); await work.releaseNext(); assert.strictEqual(lateDone, true); }); + // A rejection may carry any reason, undefined included, and must still reach the caller. + test('a run rejecting with undefined still rejects its callers', async () => { + // eslint-disable-next-line @typescript-eslint/prefer-promise-reject-errors -- the case under test + const flight = new SingleFlight(() => Promise.reject(undefined)); + let rejected = false; + await flight.run().catch(() => { rejected = true; }); + assert.strictEqual(rejected, true); + }); + test('a failed run rejects its callers, and the next request runs again', async () => { let calls = 0; const flight = new SingleFlight(async () => { diff --git a/tests/unit/testResults.test.ts b/tests/unit/testResults.test.ts index 85a85de..50474c8 100644 --- a/tests/unit/testResults.test.ts +++ b/tests/unit/testResults.test.ts @@ -2,7 +2,7 @@ import * as assert from 'assert'; import * as fs from 'fs'; import * as path from 'path'; import * as xml2js from 'xml2js'; -import { CppUnitReport, parseCppUnitReport, readCppUnitReport, reportedNames, testResults } from '../../src/testResults'; +import { type CppUnitReport, parseCppUnitReport, readCppUnitReport, reportedNames, testResults } from '../../src/testResults'; const fixturesRoot = path.resolve(__dirname, '..', '..', '..', 'tests', 'fixtures'); diff --git a/tests/unit/unityTestDiscovery.test.ts b/tests/unit/unityTestDiscovery.test.ts index 01fbfaf..fc70dd0 100644 --- a/tests/unit/unityTestDiscovery.test.ts +++ b/tests/unit/unityTestDiscovery.test.ts @@ -2,7 +2,7 @@ import * as assert from 'assert'; import * as fs from 'fs'; import * as path from 'path'; import { parseCppUnitReport } from '../../src/testResults'; -import { DiscoveredTest, discoverTests, expandRubyRange, rubyFloatToString } from '../../src/unityTestDiscovery'; +import { type DiscoveredTest, discoverTests, expandRubyRange, rubyFloatToString } from '../../src/unityTestDiscovery'; // Ceedling names every test with Unity's runner generator (auto/generate_test_runner.rb), and // the Testing view can only show a result whose name matches a discovered item. These tests pin diff --git a/tsconfig.json b/tsconfig.json index 11925af..a64437a 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -13,6 +13,7 @@ "noImplicitReturns": true, "noUnusedLocals": true, "noUnusedParameters": true, + "noUncheckedIndexedAccess": true, "removeComments": true, "skipLibCheck": true, "esModuleInterop": true,