🏗 [test-case-reporting] Implement PoC for e2e test reporter #29768
Conversation
danielrozenberg
requested review from
rcebulko
and removed request for
danielrozenberg
| runner.on(EVENT_RUN_END, async function () { | ||
| // Apparently we'll need to add a --no-exit flag when calling this | ||
| // to allow for the asynchronous reporter. | ||
| // See https://github.com/mochajs/mocha/issues/812 |
There was a problem hiding this comment.
Can we assert this flag is provided and complain if it is not, providing some explanation?
There was a problem hiding this comment.
I don't know if we can -- I don't know if using that flag is a good idea or not, I think I'd like a few more eyes on this.
There was a problem hiding this comment.
-
Does mocha support multiple reporters?
-
If yes, have you looked at existing reporters before creating a custom one? Looks like there's https://mochajs.org/#json
You might have to update the existing e2e reporter. If you do, can you move it into the e2e folder?
Yup, none of the reporters fit our needs -- the json reporter doesn't give a list of the suites of the tests, and it gives the data in a shape that the API endpoint doesn't use. |
I don't know, checking... |
Not natively -- there's a module that lets you do it (https://www.npmjs.com/package/mocha-multi), and apparently joining reporters together in one reporter isn't that complicated -- mochajs/mocha#2184 (comment) I'm good with either option, slightly prefer the latter. |
The shape of the data can be handled on the API side by inspecting the incoming object, but if the test suite info isn't available that's a sufficient reason in my mind to hand-roll our own. Is that the case? Or is the suite data in some other format? |
That is the case. The suite data is irrecoverable in the available reporters -- they use |
|
Hey @estherkim! These files were changed: |
| function ciReporter(runner, options) { | ||
| Base.call(this, runner, options); | ||
| this._karmaMimicReporter = new KarmaMimicReporter(runner, options); | ||
| this._jsonReporter = new JsonReporter(runner, options); |
There was a problem hiding this comment.
We'll probably want to skip the json reporter depending on whether a certain flag is set; I'm not familiar with this part of the codebase, what would be the cleanest way to do this?
There was a problem hiding this comment.
I suspect the same argv pattern you used for the gulp upload-report task (or whatever it was called) would work here
| @@ -163,4 +173,5 @@ e2e.flags = { | |||
| 'Options are `puppeteer` or `selenium`. Default: `selenium`', | |||
| 'headless': ' Runs the browser in headless mode', | |||
| 'debug': ' Prints debugging information while running tests', | |||
| 'report': ' Write test result report to a local file', | |||
There was a problem hiding this comment.
Didn't we use report-results or something elsewhere, or was this the same flag?
There was a problem hiding this comment.
The flag for the tasks is report; the uploading gulp task is is test-report-upload -- see #29495 (comment)
I believe it's ready. This is currently blocking a PR to add the flag to the e2e task & another PR to run the upload task. |
…ct#29768) * Implement PoC for e2e test reporter * Improve wording in comment & err message * Lint * Refactor for clarity, fix fullTitle typo * Move reporters to e2e folder, allowing imports to break for now * Fix broken imports * Rename mocha ci reporter, breaking imports * Make ci reporter that joins together the reporters * Rename KarmaMimicReporter in files * Rename karma-mimic-reporter file * Rename private _karmaMimicReporter * Simplify event insertion * Only write json report if report flag set
No description provided.