[test-case-reporting] Process e2e test reports correctly #981
Conversation
| }); | ||
| let resultList: Array<KarmaReporter.TestResult>; | ||
|
|
||
| if (job.testSuiteType === 'e2e') { |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I agree. We could then make this if(e2e)-elseif(unit|integration)-else(error-unknown)
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
I like that; could you do that in a follow-up PR once I'm gone?
| }); | ||
| let resultList: Array<KarmaReporter.TestResult>; | ||
|
|
||
| if (job.testSuiteType === 'e2e') { |
There was a problem hiding this comment.
I agree. We could then make this if(e2e)-elseif(unit|integration)-else(error-unknown)
| }); | ||
| let resultList: Array<KarmaReporter.TestResult>; | ||
|
|
||
| if (job.testSuiteType === 'e2e') { |
There was a problem hiding this comment.
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
| // Types for the JSON reports generated by Mocha | ||
| // Supposed to match the reports exactly. | ||
| namespace MochaReporter { | ||
| export interface TestResultReport { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
|
|
No description provided.