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 parsing many test suites on file #9

Closed
wants to merge 21 commits into from
Closed
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
2 changes: 2 additions & 0 deletions .github/workflows/build.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,5 @@ jobs:
run: yarn format-check
- name: Lint
run: yarn lint
- name: Unit Tests
run: yarn test
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
"format": "prettier --write **/*.ts",
"format-check": "prettier --check **/*.ts",
"lint": "eslint --ext .ts src/",
"all": "yarn type-check && yarn build && yarn format && yarn lint"
"all": "yarn type-check && yarn build && yarn format && yarn lint yarn test",
"test": "node --experimental-test-snapshots --test --import tsx src/test/*.test.ts ",
"test:update-snapshot": "node --experimental-test-snapshots --test-update-snapshots --test --import tsx src/test/*.test.ts "
},
"repository": {
"type": "git",
Expand Down
47 changes: 24 additions & 23 deletions src/junitParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ interface JUnitSuite {
}
interface JUnitReport {
testsuite?: JUnitSuite;
testsuites?: { testsuite?: JUnitSuite };
testsuites?: { testsuite?: JUnitSuite | JUnitSuite[] };
atlowChemi marked this conversation as resolved.
Show resolved Hide resolved
}

async function parseFile(file: string, projectTokenDictionaryStrs: string[]) {
Expand All @@ -70,12 +70,11 @@ async function parseFile(file: string, projectTokenDictionaryStrs: string[]) {
const data: string = fs.readFileSync(file, 'utf8');
const parser = new XMLParser({ allowBooleanAttributes: true, ignoreAttributes: false, attributeNamePrefix: '' });
const report = parser.parse(data) as Partial<JUnitReport>;

return parseSuite(report, file, projectTokenDictionaryStrs);
const testsuites = castArray(report.testsuite || report.testsuites?.testsuite);
return Promise.all(testsuites.map(async (testsuite) => parseSuite(file, projectTokenDictionaryStrs, testsuite)));
}

async function parseSuite(report: JUnitReport, fileName: string, projectTokenDictionaryStrs: string[]) {
const testsuite = report.testsuite || report.testsuites?.testsuite;
async function parseSuite(fileName: string, projectTokenDictionaryStrs: string[], testsuite?: JUnitSuite) {
const result: InternalTestResult = { fileName, name: testsuite?.name, totalCount: 0, skipped: 0, failedEvaluating: parseInt(testsuite?.['failure-evaluating'] || '0') || 0, annotations: [] };
if (!testsuite?.testcase) {
return result;
Expand Down Expand Up @@ -116,31 +115,33 @@ async function parseSuite(report: JUnitReport, fileName: string, projectTokenDic
return result;
}

async function parseTestReports(checkName: string, summary: string, reportPathsGlob: string, projectTokenDictionaryStrs: string[]) {
export async function parseTestReports(checkName: string, summary: string, reportPathsGlob: string, projectTokenDictionaryStrs: string[]) {
core.info(`Process test report for: ${reportPathsGlob} (${checkName})`);
const testResults: TestResult[] = [];

const globber = await glob.create(reportPathsGlob);
for await (const file of globber.globGenerator()) {
core.info(`Parsing report file: ${file}`);

const { totalCount, skipped, annotations, failedEvaluating, name, fileName } = await parseFile(file, projectTokenDictionaryStrs);
if (totalCount === 0) {
continue;
const suites = await parseFile(file, projectTokenDictionaryStrs);
for (const suite of suites) {
ETtestim marked this conversation as resolved.
Show resolved Hide resolved
const { totalCount, skipped, annotations, failedEvaluating, name, fileName } = suite;
if (totalCount === 0) {
continue;
}
const failed = annotations.filter(an => an.annotation_level !== 'notice').length;
const passed = totalCount - failed - skipped;
testResults.push({
summary,
checkName: name || checkName,
fileName,
totalCount,
skipped,
failedEvaluating,
annotations,
failed,
passed,
});
}
const failed = annotations.filter(an => an.annotation_level !== 'notice').length;
const passed = totalCount - failed - skipped;
testResults.push({
summary,
checkName: name || checkName,
fileName,
totalCount,
skipped,
failedEvaluating,
annotations,
failed,
passed,
});
}

return testResults;
Expand Down
18 changes: 18 additions & 0 deletions src/test/junitParser.test.ts
ETtestim marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Owner

Choose a reason for hiding this comment

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

Could you add a test ensuring we only attempt calling Testim API when key provided for given Project ID?
Also a test validating the annotations, and not only the file name

Copy link
Author

Choose a reason for hiding this comment

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

I will have another PR for this repo next sprint can we add those test then?

Copy link
Owner

Choose a reason for hiding this comment

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

What PR? which sprint?
I am confused

Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { test } from 'node:test';
import { parseTestReports } from '../junitParser.ts';

const DIRNAME = import.meta.dirname;
test('should parse junit report with multiple suites', async (t) => {
const testReports = await parseTestReports('test', 'test', `${DIRNAME}/xml/testsuites.xml`, []);
// @ts-expect-error
t.assert.strictEqual(testReports.length, 3);
// @ts-expect-error
t.assert.strictEqual(testReports[0].annotations.length, 2);
Comment on lines +7 to +10
Copy link
Owner

Choose a reason for hiding this comment

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

we could also just use assert directly and avoid // @ts-expect-error

Comment on lines +7 to +10
Copy link
Owner

Choose a reason for hiding this comment

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

Could we snapshot test the entire testReports object?

});
test('should parse junit report with 1 suite', async (t) => {
const testReports = await parseTestReports('test', 'test', `${DIRNAME}/xml/testsuite.xml`, []);
// @ts-expect-error
t.assert.strictEqual(testReports.length, 1);
// @ts-expect-error
t.assert.strictEqual(testReports[0].annotations.length, 2);
});
15 changes: 15 additions & 0 deletions src/test/junitParser.test.ts.snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
exports[`should parse junit report with 1 suite 1`] = `
1
`;

exports[`should parse junit report with 1 suite 2`] = `
2
`;

exports[`should parse junit report with 3 suites 1`] = `
3
`;

exports[`should parse junit report with 3 suites 2`] = `
2
`;
11 changes: 11 additions & 0 deletions src/test/xml/testsuite.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<testsuite name="first suite" tests="2" failures="1" timestamp="2024-06-27T08:43:18.231Z" skipped="1" failure-evaluating="1">
<testcase name="first suite first test" classname="test" time="0">
<system-out>https://app.testim.io/#/project/coco/branch/master/test/1234567A?result-id=1234567A</system-out>
<skipped/>
</testcase>
<testcase name="first suite second test" classname="test" time="0">
<system-out>https://app.testim.io/#/project/coco/branch/master/test/1234567B?result-id=1234567B</system-out>
<failure message="Step Failed: Element is not visible More info at: https://app.testim.io/#/project/coco/branch/master/test/1234567B?result-id=1234567B"/>
</testcase>
</testsuite>
29 changes: 29 additions & 0 deletions src/test/xml/testsuites.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
<testsuites>
<testsuite name="first suite" tests="2" failures="1" timestamp="2024-06-27T08:43:18.231Z" skipped="1" failure-evaluating="1">
<testcase name="first suite first test" classname="test" time="0">
<system-out>https://app.testim.io/#/project/coco/branch/master/test/1234567A?result-id=1234567A</system-out>
<skipped/>
</testcase>
<testcase name="first suite second test" classname="test" time="0">
<system-out>https://app.testim.io/#/project/coco/branch/master/test/1234567B?result-id=1234567B</system-out>
<failure message="Step Failed: Element is not visible More info at: https://app.testim.io/#/project/coco/branch/master/test/1234567B?result-id=1234567B"/>
</testcase>
</testsuite>
<testsuite name="second suite" tests="2" failures="0" timestamp="2024-06-27T08:43:18.232Z" skipped="0" failure-evaluating="0">
<testcase name="second suite first test" classname="testim.io.test">
<system-out>https://app.testim.io/#/project/coco/branch/master/test/1234567C?result-id=1234567C</system-out>
</testcase>
<testcase name="second suite second test" classname="testim.io.test">
<system-out>https://app.testim.io/#/project/coco/branch/master/test/1234567D?result-id=1234567D</system-out>
</testcase>
</testsuite>
<testsuite name="third suite" tests="2" failures="0" timestamp="2024-06-27T08:43:18.232Z" skipped="0" failure-evaluating="0">
<testcase name="third suite first test" classname="testim.io.test">
<system-out>https://app.testim.io/#/project/coco/branch/master/test/1234567E?result-id=1234567E</system-out>
</testcase>
<testcase name="third suite second test" classname="testim.io.test">
<system-out>https://app.testim.io/#/project/coco/branch/master/test/1234567F?result-id=1234567F</system-out>
</testcase>
</testsuite>
</testsuites>
6 changes: 4 additions & 2 deletions tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@
"noEmit": true,
"allowImportingTsExtensions": true,
},
"exclude": ["node_modules", "**/*.test.ts"]
}
"exclude": [
"node_modules"
]
atlowChemi marked this conversation as resolved.
Show resolved Hide resolved
}
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -466,9 +466,9 @@
integrity sha512-dRLjCWHYg4oaA77cxO64oO+7JwCwnIzkZPdrrC71jQmQtlhM556pwKo5bUzqvZndkVbeFLIIi+9TC40JNF5hNQ==

"@types/node@20":
version "20.11.9"
resolved "https://registry.yarnpkg.com/@types/node/-/node-20.11.9.tgz#959d436f20ce2ee3df897c3eaa0617c98fa70efb"
integrity sha512-CQXNuMoS/VcoAMISe5pm4JnEd1Br5jildbQEToEMQvutmv+EaQr90ry9raiudgpyDuqFiV9e4rnjSfLNq12M5w==
version "20.14.10"
resolved "https://registry.yarnpkg.com/@types/node/-/node-20.14.10.tgz#a1a218290f1b6428682e3af044785e5874db469a"
integrity sha512-MdiXf+nDuMvY0gJKxyfZ7/6UFsETO7mGKF54MVD/ekJS6HdFtpZFBgrh6Pseu64XTb2MLyFPlbW6hj8HYRQNOQ==
dependencies:
undici-types "~5.26.4"

Expand Down
Loading