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

🚀 Feature: JSON Serializable runner <-> reporter communication #3481

Closed
LinusU opened this issue Sep 19, 2018 · 8 comments
Closed

🚀 Feature: JSON Serializable runner <-> reporter communication #3481

LinusU opened this issue Sep 19, 2018 · 8 comments
Labels
status: wontfix typically a feature which won't be added, or a "bug" which is actually intended behavior type: feature enhancement proposal

Comments

@LinusU
Copy link
Contributor

LinusU commented Sep 19, 2018

I would like to bring some new life into a very old discussion, regarding the format of the communication between the runner and the reporters. I'll start by outlining my use case, then a brief coverage of the old discussion, and finally a proposed way forward.

Use case

I've recently been working on two large project which aims to normalize platforms by providing a common API (fanny-pack, wext). When working on those, I've had the need to test things in many different environments. While doing this I still want to have the ease of use, and the nice reporters, from mocha.

Currently, I'm trying to automate running tests inside a WebExtension. I have a script that will package mocha and some glue with a user-specified test file, build an extension out of it, and then use webdriver to test the extension in an actual browser.

While this currently works, I don't have a good way to get the mocha test output from inside of the extension, to the terminal that triggered the script.

The closest I can get is to use the tap reporter inside the extension, and then send that out to the script which can use tap-mocha-reporter to print the output.

The problem with this approach is that the TAP protocol have a lot less info than is needed, and the test output has incorrect timings and are missing other information. This results in a sub-par user experience.

Old discussions

This was discussed before in #492, and it seems that the maintainers had a positive attitude towards it. The problems with using TAP have also been brought up, and they are the same that I face today.

There even was a PR working towards this goal, that unfortunately never got merged: #685

Proposed road forward

I would like for there to be an easy way of splitting up the runner and the reporter into different processes. This would probably mean something like 1) working with the internals to make sure that the events passed to the runners are JSON-serializable, 2) making it easy to run the reporters standalone and just pass it a stream of events and 3) providing a way to get the raw events from mocha instead of using a reporter.

I think that we should be able to make these changes without breaking backward compatibility.

I'm very open to discussions around this, and I'm also happy to put in the work if we find a good way forward for this.

Thanks for reading ❤️

@Bamieh Bamieh added type: feature enhancement proposal status: accepting prs Mocha can use your help with this one! labels Sep 20, 2018
@Bamieh
Copy link
Contributor

Bamieh commented Sep 20, 2018

I'm generally +1 on further abstracting the reporters from the runner. CC @mochajs/core

@plroebuck
Copy link
Contributor

plroebuck commented Sep 20, 2018

The problem with this approach is that the TAP protocol have a lot less info than is needed, and the test output has incorrect timings and are missing other information.

We have updates to the TAP reporter (#3452) in process.
We also have reporter statistics refactoring (#3458) in process.
I have two additional unreleased PRs to reporters as well, providing reporterOptions for all and making several reporters file-output capable.

I have no interest in further reporter changes until the ones we already have in the pipeline are merged.

@LinusU
Copy link
Contributor Author

LinusU commented Sep 21, 2018

I have no interest in further reporter changes until the ones we already have in the pipeline are merged.

I can see the problem with having too many pull requests that touch the same code parts at the same time, that is fair. How do you feel about the problem that I stated? Do you think it would be positive to work towards improving the situation I have?

@plroebuck
Copy link
Contributor

plroebuck commented Sep 21, 2018

Having read lots of old issues/PRs, believe there are possibly too many related issues that need to be addressed first. For example:

  • Move away from cmdline-oriented "mocha.opts" configuration file.
    • Attempting to configure multiple reporters via cmdline just doesn't really scale.
    • Replace with ".mocharc.json" could help mitigate that problem, at least to some degree.
  • Way too many third-party reporters exist to just start making massive reporter API changes wholesale.

But aside from that, open to discussion.

Write down steps for some broad strokes on how you think we could achieve this.

Maybe as a first phase (multi-stage):

  • Create interface that emits Mocha events (migrating relevant code from runner).
    • Runner would inherit from interface (or via mix-in).
    • Reporter constructor would use interface rather than runner.
  • Identify and determine how to serialize moving parts of Mocha events
  • Create event-stream reporter to save serialized events to file.
  • Mocha (or another binary) could have --replay-events cmdline option (its argument a serialized events file).
    • Read file, passing events to interface, which (in turn) passes them to existing reporter.

@zbigg
Copy link

zbigg commented Oct 20, 2018

I've stumbled on this problem too as we have to run mocha-based tests in browser context and current options (mocha-chrome, `mochify) so i tried how to have mocha runtime in browser while reporter running locally on node.

I end up with:

  • in node: run fake Runner and real Reporter
  • in browser run real Runner and fake reporter which forwards all events back to node
  • only selected props of suite/test tree are serialised, but most of reporters are ok

It appeared to be quite robust, as i can have all the options of mocha and all the features of reporters - including support for mochawesome which is beast.
This solution also allowed have "node-browser-worker" communication scheme that allows running tests in Web Workers.

So, the final shameless plug here: https://github.com/zbigg/mocha-webdriver-runner

I know that serializing whole suite tree is suboptimal and could be prevented with proper event API, but maybe someone find this useful until mocha resolves issues internally.

@dotnetprofessional
Copy link

My take on this is to have two separate types of reporters. In my custom UI/Reporter I've gone with UI based reporters (ie you see the output in the console - can have but one) and Post-Reporters which take a JSON object with the full details of the test run. This is then passed to each of the registered post-reporters to do as they wish. They could output to the console or file and don't step on each other as they are run sequentially. Now having a large JSON object may pose issues with huge code bases, but I've not noticed any issues as yet.

https://github.com/dotnetprofessional/LiveDoc/blob/master/packages/livedoc-mocha/docs/Reporters.md

@JoshuaKGoldberg JoshuaKGoldberg removed the status: accepting prs Mocha can use your help with this one! label Dec 27, 2023
@JoshuaKGoldberg JoshuaKGoldberg changed the title JSON Serializable runner <-> reporter communication 🚀 Feature: JSON Serializable runner <-> reporter communication Dec 27, 2023
@JoshuaKGoldberg
Copy link
Member

Per #5027, we're trying to avoid making big changes that don't have a lot user demand. This hasn't been touched since 2019 and would be a breaking change to reporter options, I think. Closing out as wontfix.

Note that #1457 might help out here with more standardized plugin APIs. Maybe. Linking to that issue so that this one gets factored into the discussion.

If someone things I'm wrong to close this out, great! Please file a new issue with your reasoning, why the plugin APIs proposed in #1457 wouldn't be sufficient, and a link to this one. The new issue templates will prompt for the context we'd need to be able to re-triage this. We'd be happy to take a look. Cheers all! 🤎

@JoshuaKGoldberg JoshuaKGoldberg closed this as not planned Won't fix, can't repro, duplicate, stale Feb 6, 2024
@JoshuaKGoldberg JoshuaKGoldberg added the status: wontfix typically a feature which won't be added, or a "bug" which is actually intended behavior label Feb 6, 2024
@ScottFreeCode
Copy link
Contributor

This might be a silly question, but what about creating a "reporter" consuming Mocha's current API that turns the events into pure JSON data with all available info and sends those to one of:

  • a second/real reporter that takes events in pure JSON, chosen by a reporter option for this proposed Mocha-object-to-JSON "reporter"
  • an HTTP endpoint or some other connection, at the other end of which could be, again, a real reporter
  • stdout or stderr to be piped to a reporter in the same manner as TAP or redirected to a file for later analysis/reporting

In other words, sidestep the issue of changing Mocha itself:

  • The existing codebase doesn't have to be meddled with.
  • Existing third-party reporters do not get broken and will not need to be updated.
  • The new maintenance team does not have to work on adding this feature to Mocha, one or more teams of interested developers could be formed to solve it as a third party.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: wontfix typically a feature which won't be added, or a "bug" which is actually intended behavior type: feature enhancement proposal
Projects
None yet
Development

No branches or pull requests

7 participants