Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 72 additions & 69 deletions extractors/cds/tools/dist/cds-extractor.bundle.js

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions extractors/cds/tools/dist/cds-extractor.bundle.js.map

Large diffs are not rendered by default.

31 changes: 23 additions & 8 deletions extractors/cds/tools/src/cds/compiler/retry.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/** Main retry orchestration logic for CDS compilation failures. */

import { join } from 'path';

import { compileCdsToJson } from './compile';
import type {
CompilationAttempt,
Expand All @@ -16,6 +18,15 @@ import type { CdsDependencyGraph, CdsProject } from '../parser';

/**
* Add diagnostics only for tasks with `status: failed` in the {@link CdsDependencyGraph}.
*
* Compilation is project-level: a single failed task represents the whole project's
* compilation failure, even though the task's `sourceFiles` may list every CDS file
* in the project. To avoid spawning the CodeQL CLI once per source file (which can
* take tens of minutes for projects with hundreds of `.cds` files), we emit a single
* diagnostic per failed task, attached to the project's `package.json` when available
* (or the project directory otherwise), and mention the affected file count in the
* message body so that information is preserved.
*
* @param dependencyGraph The dependency graph to use as the source of truth for task status
* @param codeqlExePath Path to CodeQL executable used to add a diagnostic notification
* @param sourceRoot Source root directory to use for making file paths relative
Expand All @@ -35,14 +46,18 @@ function addCompilationDiagnosticsForFailedTasks(
const shouldAddDiagnostic = task.retryInfo?.hasBeenRetried ?? !task.retryInfo;

if (shouldAddDiagnostic) {
for (const sourceFile of task.sourceFiles) {
addCompilationDiagnostic(
sourceFile,
task.errorSummary ?? 'Compilation failed',
codeqlExePath,
sourceRoot,
);
}
// Attach the diagnostic to the project's package.json when available,
// otherwise fall back to the project directory itself.
const diagnosticPath = project.packageJson
? join(project.projectDir, 'package.json')
: project.projectDir;

const fileCount = task.sourceFiles.length;
const fileWord = fileCount === 1 ? 'file' : 'files';
const baseMessage = task.errorSummary ?? 'Compilation failed';
const messageWithCount = `${baseMessage}\n\n${fileCount} CDS ${fileWord} in this project ${fileCount === 1 ? 'was' : 'were'} not extracted.`;

addCompilationDiagnostic(diagnosticPath, messageWithCount, codeqlExePath, sourceRoot);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions extractors/cds/tools/src/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ export function addCdsIndexerDiagnostic(
}

/**
* Add a diagnostic error to the CodeQL database for a failed CDS compilation
* Add a diagnostic warning to the CodeQL database for a failed CDS compilation
* @param cdsFilePath Path to the CDS file that failed to compile
* @param errorMessage The error message from the compilation
* @param codeqlExePath Path to the CodeQL executable
Expand All @@ -167,7 +167,7 @@ export function addCompilationDiagnostic(
codeqlExePath,
'cds/compilation-failure',
'Failure to compile one or more SAP CAP CDS files',
DiagnosticSeverity.Error,
DiagnosticSeverity.Warning,
'source file',
sourceRoot,
);
Expand Down
79 changes: 74 additions & 5 deletions extractors/cds/tools/test/src/cds/compiler/retry.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/** Tests for retry orchestration logic */

import { join } from 'path';

import * as compile from '../../../../src/cds/compiler/compile';
import { orchestrateRetryAttempts } from '../../../../src/cds/compiler/retry';
import type { CompilationTask } from '../../../../src/cds/compiler/types';
Expand Down Expand Up @@ -453,10 +455,12 @@ describe('retry.ts', () => {
// Execute
orchestrateRetryAttempts(mockDependencyGraph, codeqlExePath);

// Verify diagnostics are added for failed tasks
// Verify a single project-level diagnostic is added (attached to the project's
// package.json) rather than one per source file.
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledTimes(1);
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledWith(
'/test/project/db/schema.cds',
expect.any(String),
join('test-project', 'package.json'),
expect.stringContaining('1 CDS file in this project was not extracted.'),
codeqlExePath,
'/test',
);
Expand All @@ -482,9 +486,74 @@ describe('retry.ts', () => {

orchestrateRetryAttempts(mockDependencyGraph, codeqlExePath);

// Verify diagnostics are added for tasks that were never retried (retryInfo is undefined)
// Verify a single project-level diagnostic is added for tasks that were never
// retried (retryInfo is undefined), rather than one per source file.
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledTimes(1);
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledWith(
join('test-project', 'package.json'),
expect.stringContaining('1 CDS file in this project was not extracted.'),
codeqlExePath,
'/test',
);
});

it('should emit a single diagnostic per failed task regardless of source file count', () => {
// Setup: A single failed task with many source files (simulating a large project).
const manyFiles = Array.from({ length: 250 }, (_, i) => `/test/project/db/file${i}.cds`);
const failedTask: CompilationTask = {
...mockFailedTask,
sourceFiles: manyFiles,
retryInfo: { hasBeenRetried: true, retryReason: 'Test retry' },
status: 'failed',
};
mockProject.compilationTasks = [failedTask];
mockProject.cdsFiles = manyFiles;

const tasksRequiringRetry = new Map([['test-project', [failedTask]]]);
mockValidator.identifyTasksRequiringRetry.mockReturnValue(tasksRequiringRetry);
mockPackageManager.needsFullDependencyInstallation.mockReturnValue(false);

mockCompile.compileCdsToJson.mockReturnValue({
success: false,
message: 'Compilation failed',
durationMs: 500,
});

orchestrateRetryAttempts(mockDependencyGraph, codeqlExePath);

// One diagnostic per failed task — not one per source file.
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledTimes(1);
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledWith(
join('test-project', 'package.json'),
expect.stringContaining('250 CDS files in this project were not extracted.'),
codeqlExePath,
'/test',
);
});

it('should attach the diagnostic to the project directory when no package.json exists', () => {
// Setup: A failed task on a project that has no package.json.
const failedTask = { ...mockFailedTask };
failedTask.retryInfo = { hasBeenRetried: true, retryReason: 'Test retry' };
failedTask.status = 'failed';
mockProject.compilationTasks = [failedTask];
mockProject.packageJson = undefined;

const tasksRequiringRetry = new Map([['test-project', [failedTask]]]);
mockValidator.identifyTasksRequiringRetry.mockReturnValue(tasksRequiringRetry);
mockPackageManager.needsFullDependencyInstallation.mockReturnValue(false);

mockCompile.compileCdsToJson.mockReturnValue({
success: false,
message: 'Compilation failed',
durationMs: 500,
});

orchestrateRetryAttempts(mockDependencyGraph, codeqlExePath);

expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledTimes(1);
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledWith(
'/test/project/db/schema.cds',
'test-project',
expect.any(String),
codeqlExePath,
'/test',
Expand Down
4 changes: 2 additions & 2 deletions extractors/cds/tools/test/src/diagnostics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ describe('diagnostics', () => {
'--ready-for-status-page',
'--source-id=cds/compilation-failure',
'--source-name=Failure to compile one or more SAP CAP CDS files',
'--severity=error',
'--severity=warning',
`--markdown-message=${errorMessage}`,
`--file-path=${cdsFilePath}`,
'--',
Expand Down Expand Up @@ -375,7 +375,7 @@ describe('diagnostics', () => {
expect(result).toBe(false);
expect(console.error).toHaveBeenCalledWith(
expect.stringContaining(
`ERROR: Failed to add error diagnostic for source file=${cdsFilePath}`,
`ERROR: Failed to add warning diagnostic for source file=${cdsFilePath}`,
),
);

Expand Down
Loading