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.

Sign up for GitHub

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

[test-case-reporting] Process e2e test reports correctly #981

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[test-case-reporting] Process e2e test reports correctly #981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b3513e6
Add type for mocha reporter
Rafer45 Aug 20, 2020
6a76d96
Handle e2e reports as well
Rafer45 Aug 20, 2020
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
35 changes: 24 additions & 11 deletions test-case-reporting/src/test_result_record.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import {
Build,
DB,
Job,
KarmaReporter,
MochaReporter,
PageInfo,
TestCase,
TestRun,
Expand All @@ -29,6 +31,10 @@ import {QueryBuilder} from 'knex';
import md5 from 'md5';

type QueryFunction = (q: QueryBuilder) => QueryBuilder;
interface FormattedResult extends KarmaReporter.TestResult {
Rafer45 marked this conversation as resolved.
Show resolved Hide resolved
testCaseId: string;
testCaseName: string;
}

/* eslint-disable @typescript-eslint/camelcase */
/**
Expand Down Expand Up @@ -172,17 +178,24 @@ export class TestResultRecord {
const buildId = await this.insertTravisBuild(build);
const jobId = await this.insertTravisJob(job, buildId);

const formattedResults = results.browsers
.map(({results}) => results)
.reduce((flattenedArray, array) => flattenedArray.concat(array), [])
.map(result => {
const testCaseName = this.testCaseName(result);
return {
...result,
testCaseId: md5(testCaseName),
testCaseName,
};
});
let resultList: Array<KarmaReporter.TestResult>;

if (job.testSuiteType === 'e2e') {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We could add a field to reports describing the structure of the reporter (mocha vs karma) -- I think that would be clearer than the (currently implicit) connection between e2e tests and the mocha reporter.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. We could then make this if(e2e)-elseif(unit|integration)-else(error-unknown)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, I mean changing reports from this:

{
  job: // job info
  build: // build info
  results: // results info in some shape
}

to this:

{
  job: // job info
  build: // build info
  results: // results info in some shape
  reportType: // mocha or karma
}

It's redundant but it's more readable imo, thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I would almost be in favor of

{
  reportType: // blah
  report: { job, build, results )
}

as reportType could eventually be something like github-actions-unit which might have a different report structure re: builds/jobs. Either is fine though IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like that; could you do that in a follow-up PR once I'm gone?

resultList = (results as MochaReporter.TestResultReport).testResults;
} else {
resultList = (results as KarmaReporter.TestResultReport).browsers
.map(({results}) => results)
.reduce((flattenedArray, array) => flattenedArray.concat(array), []);
}

const formattedResults: Array<FormattedResult> = resultList.map(result => {
const testCaseName = this.testCaseName(result);
return {
...result,
testCaseId: md5(testCaseName),
testCaseName,
};
});

const testCases: Array<DB.TestCase> = formattedResults.map(
({testCaseId, testCaseName}) => ({
Expand Down
13 changes: 11 additions & 2 deletions test-case-reporting/types/test-case-reporting.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ declare module 'test-case-reporting' {
/**
* Travis job types for which test results may be reported.
*/
export type TestSuiteType = 'unit' | 'integration';
export type TestSuiteType = 'unit' | 'integration' | 'e2e';

export type TestStatus = 'PASS' | 'FAIL' | 'SKIP' | 'ERROR';

Expand Down Expand Up @@ -144,6 +144,7 @@ declare module 'test-case-reporting' {
results: Array<TestResult>;
}

// TODO(980): Move this to API namespace
export interface TestResult {
description: string;
suite: Array<string>;
Expand All @@ -153,13 +154,21 @@ declare module 'test-case-reporting' {
}
}

// Types for the JSON reports generated by Mocha
// Supposed to match the reports exactly.
namespace MochaReporter {
export interface TestResultReport {
Copy link
Contributor

Choose a reason for hiding this comment

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

This could be a parameterized type TestResultReport<T>, with testResults: Array<T>. Doesn't need to happen here if it's non-trivial, esp. since it's your last day, but maybe add a TODO(rcebulko) here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd like for the KarmaReporter.TestResult type to be used by different reporters for consistency, hence why I'd like to move it to a different namespace.

testResults: Array<KarmaReporter.TestResult>;
}
}

// Types for the JSON reports POSTed by Travis
// Supposed to match the reports exactly.
namespace Travis {
export interface Report {
job: Travis.Job;
build: Travis.Build;
results: KarmaReporter.TestResultReport;
results: KarmaReporter.TestResultReport | MochaReporter.TestResultReport;
}
export interface Build {
buildNumber: string;
Expand Down