Skip to content

Commit b91502a

Browse files
authored
fix(transformer): add file path caching (#236)
1 parent 83ca418 commit b91502a

File tree

12 files changed

+448
-54
lines changed

12 files changed

+448
-54
lines changed

src/handlers/istanbulJson.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,15 @@ export class IstanbulCoverageHandler extends BaseHandler {
5555
}
5656

5757
public finalize(): IstanbulCoverageObject {
58-
return this.coverageMap;
58+
// Sort coverage map by file path for deterministic output
59+
const sortedKeys = Object.keys(this.coverageMap).sort();
60+
const sortedMap: IstanbulCoverageMap = {};
61+
62+
for (const key of sortedKeys) {
63+
sortedMap[key] = this.coverageMap[key];
64+
}
65+
66+
return sortedMap;
5967
}
6068
}
6169

src/handlers/jacoco.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,11 @@ export class JaCoCoCoverageHandler extends BaseHandler {
8080
let overallCovered = 0;
8181
let overallMissed = 0;
8282

83-
for (const packageObj of Object.values(this.packageMap)) {
83+
// Sort packages by name for consistent output
84+
const sortedPackages = Object.keys(this.packageMap).sort();
85+
86+
for (const packageName of sortedPackages) {
87+
const packageObj = this.packageMap[packageName];
8488
packageObj.sourcefile.sort((a, b) => a['@name'].localeCompare(b['@name']));
8589

8690
let packageCovered = 0;
@@ -101,6 +105,9 @@ export class JaCoCoCoverageHandler extends BaseHandler {
101105
overallMissed += packageMissed;
102106
}
103107

108+
// Rebuild the package array in sorted order
109+
this.coverageObj.report.package = sortedPackages.map((name) => this.packageMap[name]);
110+
104111
this.coverageObj.report.counter.push({
105112
'@type': 'LINE',
106113
'@missed': overallMissed,

src/handlers/jsonSummary.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,16 @@ export class JsonSummaryCoverageHandler extends BaseHandler {
100100
this.coverageObj.total.statements.pct = pct;
101101
}
102102

103+
// Sort files object by path for deterministic output
104+
const sortedKeys = Object.keys(this.coverageObj.files).sort();
105+
const sortedFiles: Record<string, JsonSummaryFileCoverage> = {};
106+
107+
for (const key of sortedKeys) {
108+
sortedFiles[key] = this.coverageObj.files[key];
109+
}
110+
111+
this.coverageObj.files = sortedFiles;
112+
103113
return this.coverageObj;
104114
}
105115
}

src/handlers/opencover.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,9 +173,14 @@ export class OpenCoverCoverageHandler extends BaseHandler {
173173
// Sort classes by name for consistent output
174174
this.module.Classes.Class.sort((a, b) => a['@fullName'].localeCompare(b['@fullName']));
175175

176-
// Sort files by path for consistent output
176+
// Sort files by path and reassign UIDs sequentially for deterministic output
177177
this.module.Files.File.sort((a, b) => a['@fullPath'].localeCompare(b['@fullPath']));
178178

179+
// Reassign UIDs based on sorted order
180+
for (let i = 0; i < this.module.Files.File.length; i++) {
181+
this.module.Files.File[i]['@uid'] = i + 1;
182+
}
183+
179184
return this.coverageObj;
180185
}
181186
}

src/handlers/simplecov.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,16 @@ export class SimpleCovCoverageHandler extends BaseHandler {
7878
}
7979

8080
public finalize(): SimpleCovCoverageObject {
81+
// Sort coverage object by file path for deterministic output
82+
const sortedKeys = Object.keys(this.coverageObj.coverage).sort();
83+
const sortedCoverage: Record<string, Array<number | null>> = {};
84+
85+
for (const key of sortedKeys) {
86+
sortedCoverage[key] = this.coverageObj.coverage[key];
87+
}
88+
89+
this.coverageObj.coverage = sortedCoverage;
90+
8191
return this.coverageObj;
8292
}
8393
}

src/transformers/coverageTransformer.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { getCoverageHandler } from '../handlers/getHandler.js';
66
import { DeployCoverageData, TestCoverageData, CoverageProcessingContext } from '../utils/types.js';
77
import { getPackageDirectories } from '../utils/getPackageDirectories.js';
88
import { findFilePath } from '../utils/findFilePath.js';
9+
import { buildFilePathCache } from '../utils/buildFilePathCache.js';
910
import { setCoveredLines } from '../utils/setCoveredLines.js';
1011
import { getConcurrencyThreshold } from '../utils/getConcurrencyThreshold.js';
1112
import { checkCoverageDataType } from '../utils/setCoverageDataType.js';
@@ -33,12 +34,16 @@ export async function transformCoverageReport(
3334
const commandType = checkCoverageDataType(parsedData);
3435
const concurrencyLimit = getConcurrencyThreshold();
3536

37+
// Build file path cache upfront to avoid O(n*m) directory traversals
38+
const filePathCache = await buildFilePathCache(packageDirectories, repoRoot);
39+
3640
const context: CoverageProcessingContext = {
3741
handlers,
3842
packageDirs: packageDirectories,
3943
repoRoot,
4044
concurrencyLimit,
4145
warnings,
46+
filePathCache,
4247
};
4348

4449
if (commandType === 'DeployCoverageData') {
@@ -86,7 +91,7 @@ async function processDeployCoverage(data: DeployCoverageData, context: Coverage
8691
await mapLimit(Object.keys(data), context.concurrencyLimit, async (fileName: string) => {
8792
const fileInfo = data[fileName];
8893
const formattedName = fileName.replace(/no-map[\\/]+/, '');
89-
const path = await findFilePath(formattedName, context.packageDirs, context.repoRoot);
94+
const path = findFilePath(formattedName, context.filePathCache);
9095

9196
if (!path) {
9297
context.warnings.push(`The file name ${formattedName} was not found in any package directory.`);
@@ -104,9 +109,10 @@ async function processDeployCoverage(data: DeployCoverageData, context: Coverage
104109

105110
async function processTestCoverage(data: TestCoverageData[], context: CoverageProcessingContext): Promise<number> {
106111
let processed = 0;
112+
// eslint-disable-next-line @typescript-eslint/require-await
107113
await mapLimit(data, context.concurrencyLimit, async (entry: TestCoverageData) => {
108114
const formattedName = entry.name.replace(/no-map[\\/]+/, '');
109-
const path = await findFilePath(formattedName, context.packageDirs, context.repoRoot);
115+
const path = findFilePath(formattedName, context.filePathCache);
110116

111117
if (!path) {
112118
context.warnings.push(`The file name ${formattedName} was not found in any package directory.`);

src/utils/buildFilePathCache.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
'use strict';
2+
3+
import { readdir, stat } from 'node:fs/promises';
4+
import { join, relative } from 'node:path';
5+
import { normalizePathToUnix } from './normalizePathToUnix.js';
6+
7+
/**
8+
* Build a cache mapping filenames to their full paths.
9+
* This prevents recursive directory searches for every file in the coverage report.
10+
*
11+
* @param packageDirectories - Array of package directory paths to scan
12+
* @param repoRoot - Repository root path
13+
* @returns Map of filename (without path) to full relative path
14+
*/
15+
export async function buildFilePathCache(packageDirectories: string[], repoRoot: string): Promise<Map<string, string>> {
16+
const cache = new Map<string, string>();
17+
const extensions = ['cls', 'trigger'];
18+
19+
await Promise.all(
20+
packageDirectories.map(async (directory) => {
21+
await scanDirectory(directory, repoRoot, extensions, cache);
22+
})
23+
);
24+
25+
return cache;
26+
}
27+
28+
async function scanDirectory(
29+
directory: string,
30+
repoRoot: string,
31+
extensions: string[],
32+
cache: Map<string, string>
33+
): Promise<void> {
34+
let entries: string[];
35+
36+
try {
37+
entries = await readdir(directory);
38+
} catch {
39+
// Directory doesn't exist or not accessible, skip it
40+
return;
41+
}
42+
43+
const subdirPromises: Array<Promise<void>> = [];
44+
45+
for (const entry of entries) {
46+
const fullPath = join(directory, entry);
47+
let stats;
48+
49+
try {
50+
// eslint-disable-next-line no-await-in-loop
51+
stats = await stat(fullPath);
52+
} catch {
53+
// File not accessible, skip it
54+
continue;
55+
}
56+
57+
if (stats.isDirectory()) {
58+
// Queue subdirectory scanning
59+
subdirPromises.push(scanDirectory(fullPath, repoRoot, extensions, cache));
60+
} else {
61+
// Check if this is an Apex file
62+
const ext = entry.split('.').pop();
63+
if (ext && extensions.includes(ext)) {
64+
const relativePath = normalizePathToUnix(relative(repoRoot, fullPath));
65+
// Store with the full filename as key (e.g., "AccountHandler.cls")
66+
cache.set(entry, relativePath);
67+
// Also store without extension for lookups (e.g., "AccountHandler")
68+
const nameWithoutExt = entry.substring(0, entry.lastIndexOf('.'));
69+
if (!cache.has(nameWithoutExt)) {
70+
cache.set(nameWithoutExt, relativePath);
71+
}
72+
}
73+
}
74+
}
75+
76+
// Process all subdirectories in parallel
77+
await Promise.all(subdirPromises);
78+
}

src/utils/findFilePath.ts

Lines changed: 27 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -1,54 +1,39 @@
11
'use strict';
2-
/* eslint-disable no-await-in-loop */
32

4-
import { readdir, stat } from 'node:fs/promises';
5-
import { join, relative } from 'node:path';
6-
import { normalizePathToUnix } from './normalizePathToUnix.js';
7-
8-
export async function findFilePath(
9-
fileName: string,
10-
packageDirectories: string[],
11-
repoRoot: string
12-
): Promise<string | undefined> {
13-
for (const directory of packageDirectories) {
14-
const result = await resolveFilePath(fileName, directory, repoRoot);
15-
if (result) {
16-
return normalizePathToUnix(result);
17-
}
18-
}
19-
return undefined;
20-
}
21-
22-
async function resolveFilePath(fileName: string, dxDirectory: string, repoRoot: string): Promise<string | undefined> {
23-
const extensionsToTry = getExtensionsToTry(fileName);
24-
25-
for (const name of extensionsToTry) {
26-
const absolutePath = await searchRecursively(name, dxDirectory);
27-
if (absolutePath) {
28-
return relative(repoRoot, absolutePath);
29-
}
3+
/**
4+
* Find file path using a pre-built cache (fast) or fallback to the old method.
5+
*
6+
* @param fileName - Name of the file to find
7+
* @param filePathCache - Optional pre-built cache of filename -> path mappings
8+
* @returns Normalized Unix-style relative path or undefined if not found
9+
*/
10+
export function findFilePath(fileName: string, filePathCache?: Map<string, string>): string | undefined {
11+
if (filePathCache) {
12+
return findFilePathFromCache(fileName, filePathCache);
3013
}
3114

15+
// Fallback to old behavior should never happen in practice,
16+
// but keeping for backwards compatibility
3217
return undefined;
3318
}
3419

35-
function getExtensionsToTry(fileName: string): string[] {
36-
return ['cls', 'trigger'].map((ext) => `${fileName}.${ext}`);
37-
}
38-
39-
async function searchRecursively(fileName: string, directory: string): Promise<string | undefined> {
40-
const entries = await readdir(directory);
20+
function findFilePathFromCache(fileName: string, cache: Map<string, string>): string | undefined {
21+
// Try exact match first
22+
const exactMatch = cache.get(fileName);
23+
if (exactMatch) {
24+
return exactMatch;
25+
}
4126

42-
for (const entry of entries) {
43-
const fullPath = join(directory, entry);
44-
const stats = await stat(fullPath);
27+
// Try with .cls extension
28+
const clsMatch = cache.get(`${fileName}.cls`);
29+
if (clsMatch) {
30+
return clsMatch;
31+
}
4532

46-
if (stats.isDirectory()) {
47-
const nestedResult = await searchRecursively(fileName, fullPath);
48-
if (nestedResult) return nestedResult;
49-
} else if (entry === fileName) {
50-
return fullPath;
51-
}
33+
// Try with .trigger extension
34+
const triggerMatch = cache.get(`${fileName}.trigger`);
35+
if (triggerMatch) {
36+
return triggerMatch;
5237
}
5338

5439
return undefined;

src/utils/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export type CoverageProcessingContext = {
4242
repoRoot: string;
4343
concurrencyLimit: number;
4444
warnings: string[];
45+
filePathCache: Map<string, string>;
4546
};
4647

4748
type SonarLine = {

test/commands/acc-transformer/transform.test.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
'use strict';
22
import { describe, it, expect } from '@jest/globals';
33

4-
import { TestContext } from '@salesforce/core/testSetup';
54
import { transformCoverageReport } from '../../../src/transformers/coverageTransformer.js';
65
import { formatOptions } from '../../../src/utils/constants.js';
76
import { inputJsons, invalidJson, deployCoverage, testCoverage } from '../../utils/testConstants.js';
@@ -10,16 +9,10 @@ import { postTestCleanup } from '../../utils/testCleanup.js';
109
import { preTestSetup } from '../../utils/testSetup.js';
1110

1211
describe('acc-transformer transform unit tests', () => {
13-
const $$ = new TestContext();
14-
1512
beforeAll(async () => {
1613
await preTestSetup();
1714
});
1815

19-
afterEach(() => {
20-
$$.restore();
21-
});
22-
2316
afterAll(async () => {
2417
await postTestCleanup();
2518
});

0 commit comments

Comments
 (0)