Skip to content

Commit c887110

Browse files
authored
Merge pull request #376 from mihai-herda-SAP/fix_excessive_debug_info
CDS Extractor: Fix excessive diagnostic info
2 parents f133c7d + 412a8a7 commit c887110

6 files changed

Lines changed: 177 additions & 90 deletions

File tree

extractors/cds/tools/dist/cds-extractor.bundle.js

Lines changed: 72 additions & 69 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

extractors/cds/tools/dist/cds-extractor.bundle.js.map

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

extractors/cds/tools/src/cds/compiler/retry.ts

Lines changed: 23 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
/** Main retry orchestration logic for CDS compilation failures. */
22

3+
import { join } from 'path';
4+
35
import { compileCdsToJson } from './compile';
46
import type {
57
CompilationAttempt,
@@ -16,6 +18,15 @@ import type { CdsDependencyGraph, CdsProject } from '../parser';
1618

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

3748
if (shouldAddDiagnostic) {
38-
for (const sourceFile of task.sourceFiles) {
39-
addCompilationDiagnostic(
40-
sourceFile,
41-
task.errorSummary ?? 'Compilation failed',
42-
codeqlExePath,
43-
sourceRoot,
44-
);
45-
}
49+
// Attach the diagnostic to the project's package.json when available,
50+
// otherwise fall back to the project directory itself.
51+
const diagnosticPath = project.packageJson
52+
? join(project.projectDir, 'package.json')
53+
: project.projectDir;
54+
55+
const fileCount = task.sourceFiles.length;
56+
const fileWord = fileCount === 1 ? 'file' : 'files';
57+
const baseMessage = task.errorSummary ?? 'Compilation failed';
58+
const messageWithCount = `${baseMessage}\n\n${fileCount} CDS ${fileWord} in this project ${fileCount === 1 ? 'was' : 'were'} not extracted.`;
59+
60+
addCompilationDiagnostic(diagnosticPath, messageWithCount, codeqlExePath, sourceRoot);
4661
}
4762
}
4863
}

extractors/cds/tools/src/diagnostics.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ export function addCdsIndexerDiagnostic(
148148
}
149149

150150
/**
151-
* Add a diagnostic error to the CodeQL database for a failed CDS compilation
151+
* Add a diagnostic warning to the CodeQL database for a failed CDS compilation
152152
* @param cdsFilePath Path to the CDS file that failed to compile
153153
* @param errorMessage The error message from the compilation
154154
* @param codeqlExePath Path to the CodeQL executable
@@ -167,7 +167,7 @@ export function addCompilationDiagnostic(
167167
codeqlExePath,
168168
'cds/compilation-failure',
169169
'Failure to compile one or more SAP CAP CDS files',
170-
DiagnosticSeverity.Error,
170+
DiagnosticSeverity.Warning,
171171
'source file',
172172
sourceRoot,
173173
);

extractors/cds/tools/test/src/cds/compiler/retry.test.ts

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
/** Tests for retry orchestration logic */
22

3+
import { join } from 'path';
4+
35
import * as compile from '../../../../src/cds/compiler/compile';
46
import { orchestrateRetryAttempts } from '../../../../src/cds/compiler/retry';
57
import type { CompilationTask } from '../../../../src/cds/compiler/types';
@@ -453,10 +455,12 @@ describe('retry.ts', () => {
453455
// Execute
454456
orchestrateRetryAttempts(mockDependencyGraph, codeqlExePath);
455457

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

483487
orchestrateRetryAttempts(mockDependencyGraph, codeqlExePath);
484488

485-
// Verify diagnostics are added for tasks that were never retried (retryInfo is undefined)
489+
// Verify a single project-level diagnostic is added for tasks that were never
490+
// retried (retryInfo is undefined), rather than one per source file.
491+
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledTimes(1);
492+
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledWith(
493+
join('test-project', 'package.json'),
494+
expect.stringContaining('1 CDS file in this project was not extracted.'),
495+
codeqlExePath,
496+
'/test',
497+
);
498+
});
499+
500+
it('should emit a single diagnostic per failed task regardless of source file count', () => {
501+
// Setup: A single failed task with many source files (simulating a large project).
502+
const manyFiles = Array.from({ length: 250 }, (_, i) => `/test/project/db/file${i}.cds`);
503+
const failedTask: CompilationTask = {
504+
...mockFailedTask,
505+
sourceFiles: manyFiles,
506+
retryInfo: { hasBeenRetried: true, retryReason: 'Test retry' },
507+
status: 'failed',
508+
};
509+
mockProject.compilationTasks = [failedTask];
510+
mockProject.cdsFiles = manyFiles;
511+
512+
const tasksRequiringRetry = new Map([['test-project', [failedTask]]]);
513+
mockValidator.identifyTasksRequiringRetry.mockReturnValue(tasksRequiringRetry);
514+
mockPackageManager.needsFullDependencyInstallation.mockReturnValue(false);
515+
516+
mockCompile.compileCdsToJson.mockReturnValue({
517+
success: false,
518+
message: 'Compilation failed',
519+
durationMs: 500,
520+
});
521+
522+
orchestrateRetryAttempts(mockDependencyGraph, codeqlExePath);
523+
524+
// One diagnostic per failed task — not one per source file.
525+
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledTimes(1);
526+
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledWith(
527+
join('test-project', 'package.json'),
528+
expect.stringContaining('250 CDS files in this project were not extracted.'),
529+
codeqlExePath,
530+
'/test',
531+
);
532+
});
533+
534+
it('should attach the diagnostic to the project directory when no package.json exists', () => {
535+
// Setup: A failed task on a project that has no package.json.
536+
const failedTask = { ...mockFailedTask };
537+
failedTask.retryInfo = { hasBeenRetried: true, retryReason: 'Test retry' };
538+
failedTask.status = 'failed';
539+
mockProject.compilationTasks = [failedTask];
540+
mockProject.packageJson = undefined;
541+
542+
const tasksRequiringRetry = new Map([['test-project', [failedTask]]]);
543+
mockValidator.identifyTasksRequiringRetry.mockReturnValue(tasksRequiringRetry);
544+
mockPackageManager.needsFullDependencyInstallation.mockReturnValue(false);
545+
546+
mockCompile.compileCdsToJson.mockReturnValue({
547+
success: false,
548+
message: 'Compilation failed',
549+
durationMs: 500,
550+
});
551+
552+
orchestrateRetryAttempts(mockDependencyGraph, codeqlExePath);
553+
554+
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledTimes(1);
486555
expect(mockDiagnostics.addCompilationDiagnostic).toHaveBeenCalledWith(
487-
'/test/project/db/schema.cds',
556+
'test-project',
488557
expect.any(String),
489558
codeqlExePath,
490559
'/test',

extractors/cds/tools/test/src/diagnostics.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ describe('diagnostics', () => {
344344
'--ready-for-status-page',
345345
'--source-id=cds/compilation-failure',
346346
'--source-name=Failure to compile one or more SAP CAP CDS files',
347-
'--severity=error',
347+
'--severity=warning',
348348
`--markdown-message=${errorMessage}`,
349349
`--file-path=${cdsFilePath}`,
350350
'--',
@@ -375,7 +375,7 @@ describe('diagnostics', () => {
375375
expect(result).toBe(false);
376376
expect(console.error).toHaveBeenCalledWith(
377377
expect.stringContaining(
378-
`ERROR: Failed to add error diagnostic for source file=${cdsFilePath}`,
378+
`ERROR: Failed to add warning diagnostic for source file=${cdsFilePath}`,
379379
),
380380
);
381381

0 commit comments

Comments
 (0)