Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: modify large queries use query more to fetch all needed records #338

Merged
merged 12 commits into from
Dec 8, 2023
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,15 @@
"bugs": "https://github.com/forcedotcom/salesforcedx-apex/issues",
"main": "lib/src/index.js",
"dependencies": {
"@salesforce/core": "^5.3.18",
"@salesforce/core": "^5.3.20",
"@salesforce/kit": "^3.0.15",
"@types/istanbul-reports": "^3.0.4",
"faye": "1.4.0",
"glob": "^10.3.10",
"istanbul-lib-coverage": "^3.2.2",
"istanbul-lib-report": "^3.0.1",
"istanbul-reports": "^3.1.6",
"jsforce": "^2.0.0-beta.28"
"jsforce": "^2.0.0-beta.29"
},
"devDependencies": {
"@commitlint/config-conventional": "^18.1.0",
Expand Down Expand Up @@ -86,4 +86,4 @@
"path": "./node_modules/cz-conventional-changelog"
}
}
}
}
2 changes: 1 addition & 1 deletion src/execute/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { soapTemplate, action, xmlCharMap } from './types';
import * as util from 'util';

function escapeXml(data: string): string {
return data.replace(/[<>&'"]/g, (char) => {
return data.replace(/[<>&'"]/g, char => {
return xmlCharMap[char];
});
}
Expand Down
3 changes: 2 additions & 1 deletion src/i18n/i18n.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,5 +91,6 @@ export const messages = {
covIdFormatErr: 'Cannot specify code coverage with a TestRunId result',
startHandshake: 'Attempting StreamingClient handshake',
finishHandshake: 'Finished StreamingClient handshake',
subscribeStarted: 'Subscribing to ApexLog events'
subscribeStarted: 'Subscribing to ApexLog events',
invalidCountQueryResult: 'Invalid query result for count query ("%s")'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this a user facing error? what will the user do if one sees this error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it is customer facing, but unlikely to fail. I imagine the error would become part of an issue report.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this message is not needed anymore I believe?

};
2 changes: 1 addition & 1 deletion src/i18n/localization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export class Message implements LocalizationProvider {
possibleLabel = `${MISSING_LABEL_MSG} ${label}`;

if (Array.isArray(args) && args.length >= 1) {
args.forEach((arg) => {
args.forEach(arg => {
possibleLabel += ` (${arg})`;
});
}
Expand Down
6 changes: 3 additions & 3 deletions src/logs/logService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class LogService {

if (typeof options.numberOfLogs === 'number') {
const logIdRecordList = await this.getLogRecords(options.numberOfLogs);
return logIdRecordList.map((logRecord) => logRecord.Id);
return logIdRecordList.map(logRecord => logRecord.Id);
}
return [options.logId];
}
Expand All @@ -65,7 +65,7 @@ export class LogService {
public async getLogs(options: ApexLogGetOptions): Promise<LogResult[]> {
const logIdList = await this.getLogIds(options);
const logPaths: string[] = [];
const connectionRequests = logIdList.map(async (id) => {
const connectionRequests = logIdList.map(async id => {
const url = `${this.connection.tooling._baseUrl()}/sobjects/ApexLog/${id}/Body`;
const logRecord = await this.toolingRequest(url);
if (options.outputDir) {
Expand All @@ -85,7 +85,7 @@ export class LogService {
return logMap;
}

return logs.map((log) => {
return logs.map(log => {
return { log };
});
}
Expand Down
13 changes: 6 additions & 7 deletions src/reporters/coverageReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export class CoverageReporter {
coverageMap: this.coverageMap
});
const formats = this.options?.reportFormats || ['text-summary'];
formats.forEach((format) => {
formats.forEach(format => {
const report = reports.create(
format,
this.options?.reportOptions[format] || DefaultReportOptions[format]
Expand All @@ -125,23 +125,22 @@ export class CoverageReporter {
const coverageMap = libCoverage.createCoverageMap();
this.coverage.records.forEach(
(record: ApexCodeCoverageRecord | ApexCodeCoverageAggregateRecord) => {
const fileCoverageData: libCoverage.FileCoverageData =
{} as libCoverage.FileCoverageData;
const fileCoverageData: libCoverage.FileCoverageData = {} as libCoverage.FileCoverageData;
const fileRegEx = new RegExp(
`${record.ApexClassOrTrigger.Name}\.(cls|trigger)`
);
fileCoverageData.fnMap = {};
fileCoverageData.branchMap = {};
fileCoverageData.path = path.join(
this.sourceDir,
pathsToFiles.find((file) => fileRegEx.test(file)) ||
pathsToFiles.find(file => fileRegEx.test(file)) ||
record.ApexClassOrTrigger.Name
);
fileCoverageData.f = {};
fileCoverageData.b = {};
fileCoverageData.s = [
...record.Coverage.coveredLines.map((line) => [line, 1]),
...record.Coverage.uncoveredLines.map((line) => [line, 0])
...record.Coverage.coveredLines.map(line => [line, 1]),
...record.Coverage.uncoveredLines.map(line => [line, 0])
]
.map(([line, covered]) => [Number(line).toString(10), covered])
.reduce((acc, [line, value]) => {
Expand All @@ -160,7 +159,7 @@ export class CoverageReporter {
...record.Coverage.uncoveredLines
]
.sort()
.map((line) => {
.map(line => {
const statement: libCoverage.Range = {
start: {
line,
Expand Down
2 changes: 1 addition & 1 deletion src/reporters/humanReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export class HumanReporter {
: elem.message;

if (elem.perClassCoverage) {
elem.perClassCoverage.forEach((perClassCov) => {
elem.perClassCoverage.forEach(perClassCov => {
testRowArray.push({
name: elem.fullName,
coveredClassName: perClassCov.apexClassOrTriggerName,
Expand Down
10 changes: 5 additions & 5 deletions src/reporters/tapReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,16 +24,16 @@ export class TapReporter {

let out = '';
out = out.concat(`1..${testPointCount}\n`);
results.forEach((testPoint) => {
results.forEach(testPoint => {
out = out.concat(
`${testPoint.outcome} ${testPoint.testNumber} ${testPoint.description}\n`
);
testPoint.diagnostics.forEach((s) => {
testPoint.diagnostics.forEach(s => {
out = out.concat(`# ${s}\n`);
});
});

epilog?.forEach((c) => {
epilog?.forEach(c => {
out = out.concat(`# ${c}\n`);
});
return out;
Expand Down Expand Up @@ -61,7 +61,7 @@ export class TapReporter {
if (testResult.message) {
const startsWithNewlineRegex = new RegExp(/^[/\r\n|\r|\n][\w]*/gim);
if (startsWithNewlineRegex.test(testResult.message)) {
testResult.message.split(/\r\n|\r|\n/g).forEach((msg) => {
testResult.message.split(/\r\n|\r|\n/g).forEach(msg => {
if (msg && msg.length > 0) {
message.push(msg.trim());
}
Expand All @@ -74,7 +74,7 @@ export class TapReporter {
}

if (testResult.stackTrace) {
testResult.stackTrace.split('\n').forEach((line) => {
testResult.stackTrace.split('\n').forEach(line => {
message.push(line);
});
}
Expand Down
8 changes: 4 additions & 4 deletions src/streaming/streamingClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export class Deferred<T> {
public promise: Promise<T>;
public resolve: Function;
constructor() {
this.promise = new Promise((resolve) => (this.resolve = resolve));
this.promise = new Promise(resolve => (this.resolve = resolve));
}
}

Expand Down Expand Up @@ -149,7 +149,7 @@ export class StreamingClient {
}

public handshake(): Promise<void> {
return new Promise((resolve) => {
return new Promise(resolve => {
this.client.handshake(() => {
resolve();
});
Expand Down Expand Up @@ -188,7 +188,7 @@ export class StreamingClient {

if (action) {
action()
.then((id) => {
.then(id => {
this.subscribedTestRunId = id;
this.subscribedTestRunIdDeferred.resolve(id);

Expand All @@ -206,7 +206,7 @@ export class StreamingClient {
}, RetrieveResultsInterval);
}
})
.catch((e) => {
.catch(e => {
this.disconnect();
clearInterval(intervalId);
subscriptionReject(e);
Expand Down
59 changes: 36 additions & 23 deletions src/tests/asyncTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,17 @@ import {
TestResult,
TestRunIdResult
} from './types';
import { calculatePercentage, isValidTestRunID } from './utils';
import {
calculatePercentage,
isValidTestRunID,
queryAll,
verifyCountQueries
} from './utils';
import * as util from 'util';
import { QUERY_RECORD_LIMIT } from './constants';
import { CodeCoverage } from './codeCoverage';
import { HttpRequest } from 'jsforce';
import { HttpRequest, QueryResult } from 'jsforce';
import { OrgConfigProperties } from '@salesforce/core/lib/org/orgConfigProperties';

export class AsyncTests {
public readonly connection: Connection;
Expand Down Expand Up @@ -208,8 +214,11 @@ export class AsyncTests {
const apexTestResults = await this.getAsyncTestResults(
asyncRunResult.queueItem
);
const { apexTestClassIdSet, testResults, globalTests } =
await this.buildAsyncTestResults(apexTestResults);
const {
apexTestClassIdSet,
testResults,
globalTests
} = await this.buildAsyncTestResults(apexTestResults);

let outcome = testRunSummary.Status;
if (globalTests.failed > 0) {
Expand Down Expand Up @@ -245,15 +254,16 @@ export class AsyncTests {
};

if (codeCoverage) {
const perClassCovMap =
await this.codecoverage.getPerClassCodeCoverage(apexTestClassIdSet);
const perClassCovMap = await this.codecoverage.getPerClassCodeCoverage(
apexTestClassIdSet
);

result.tests.forEach((item) => {
result.tests.forEach(item => {
const keyCodeCov = `${item.apexClass.id}-${item.methodName}`;
const perClassCov = perClassCovMap.get(keyCodeCov);
// Skipped test is not in coverage map, check to see if perClassCov exists first
if (perClassCov) {
perClassCov.forEach((classCov) =>
perClassCov.forEach(classCov =>
coveredApexClassIdSet.add(classCov.apexClassOrTriggerId)
);
item.perClassCoverage = perClassCov;
Expand All @@ -265,17 +275,21 @@ export class AsyncTests {
value: 'queryingForAggregateCodeCoverage',
message: nls.localize('queryingForAggregateCodeCoverage')
});
const { codeCoverageResults, totalLines, coveredLines } =
await this.codecoverage.getAggregateCodeCoverage(coveredApexClassIdSet);
const {
codeCoverageResults,
totalLines,
coveredLines
} = await this.codecoverage.getAggregateCodeCoverage(
coveredApexClassIdSet
);
result.codecoverage = codeCoverageResults;
result.summary.totalLines = totalLines;
result.summary.coveredLines = coveredLines;
result.summary.testRunCoverage = calculatePercentage(
coveredLines,
totalLines
);
result.summary.orgWideCoverage =
await this.codecoverage.getOrgWideCoverage();
result.summary.orgWideCoverage = await this.codecoverage.getOrgWideCoverage();
}

return result;
Expand All @@ -291,25 +305,23 @@ export class AsyncTests {
'ApexClass.Id, ApexClass.Name, ApexClass.NamespacePrefix ';
apexTestResultQuery += 'FROM ApexTestResult WHERE QueueItemId IN (%s)';

const apexResultIds = testQueueResult.records.map((record) => record.Id);
const apexResultIds = testQueueResult.records.map(record => record.Id);

// iterate thru ids, create query with id, & compare query length to char limit
const queries: string[] = [];
for (let i = 0; i < apexResultIds.length; i += QUERY_RECORD_LIMIT) {
const recordSet: string[] = apexResultIds
.slice(i, i + QUERY_RECORD_LIMIT)
.map((id) => `'${id}'`);
.map(id => `'${id}'`);
const query: string = util.format(
apexTestResultQuery,
recordSet.join(',')
);
queries.push(query);
}

const queryPromises = queries.map((query) => {
return this.connection.tooling.query<ApexTestResultRecord>(query, {
autoFetch: true
});
const queryPromises = queries.map(query => {
return queryAll(this.connection, query, true);
});
const apexTestResults = await Promise.all(queryPromises);
return apexTestResults as ApexTestResult[];
Expand All @@ -334,7 +346,7 @@ export class AsyncTests {
// Iterate over test results, format and add them as results.tests
const testResults: ApexTestResultData[] = [];
for (const result of apexTestResults) {
result.records.forEach((item) => {
result.records.forEach(item => {
switch (item.Outcome) {
case ApexTestResultOutcome.Pass:
passed++;
Expand Down Expand Up @@ -402,10 +414,11 @@ export class AsyncTests {
testRunId
});

const testQueueItems =
await this.connection.tooling.query<ApexTestQueueItemRecord>(
`SELECT Id, Status FROM ApexTestQueueItem WHERE ParentJobId = '${testRunId}'`
);
const testQueueItems = await this.connection.tooling.query<
ApexTestQueueItemRecord
>(
`SELECT Id, Status FROM ApexTestQueueItem WHERE ParentJobId = '${testRunId}'`
);

for (const record of testQueueItems.records) {
record.Status = ApexTestQueueItemStatus.Aborted;
Expand Down
Loading