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

Add JSON Reporter output to object similar to #2651 #4822

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ashleyfrieze
Copy link

Description of the Change

Add to the existing configuration options for the JSON reporter by allowing a target object to be provided to be populated with test results.

This follows on from the existing ability to specify an output file, but suits situations where files cannot be used to communicate results to the outside world.

Alternate Designs

We could make a duplicate of the JSON reporter, purely for writing to objects. A workaround is to hack in an alternative of the JSON reporter into the reporters in the runner. However, this is such a small addition to the core, and entirely in keeping with the behaviour of producing a JavaScript model of the test results, that it seems to belong in core.

Why should this be in core?

This is a minor, useful change to an existing reporter.

Benefits

When executing mocha within a browser test, or explicitly, there's a way to capture the outcome of the test for reporting.

The use case that drove this is where I'm using mocha as the test framework, running it in a virtual browser, driving the tests from a Java application and need to gather the test results from Mocha. I don't have the ability to send them to a file from this environment, but being able to put them into a variable means I can pull the results out at the end.

Possible Drawbacks

This is a minor alternative to the existing options of the reporter. It should be dormant unless used.

Applicable issues

@github-actions
Copy link

This PR hasn't had any recent activity, and I'm labeling it stale. Remove the label or comment or this PR will be closed in 14 days. Thanks for contributing to Mocha!

@github-actions github-actions bot added the stale this has been inactive for a while... label May 30, 2022
@github-actions github-actions bot closed this Jun 13, 2022
@ashleyfrieze
Copy link
Author

Disappointing.

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Dec 2, 2023

👋 coming back to this @ashleyfrieze, are you still interested in working on this PR? As of #5027 there are maintainers who can review it now. No worries if you no longer have time - but if you do that'd be great!

Note that I don't see any linked issues. The contributing guidelines right now ask for that so that folks can discuss the change first. We'll continue to require that after #5038. Was there any impetus for this PR besides what's in the description?

@ashleyfrieze
Copy link
Author

I think I left a worked example. I can consult on it but may not have much time to finesse it. It seemed like an omission not to be able to do this, and made running mocha inside HTMLUnit in Java a bit tricky.

What do you need to bring this PR in? IIRC it was pretty minor

@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Dec 2, 2023

To be honest I'm not yet familiar with the reporters side of Mocha. The three of us new maintainers still need to ramp up on the project in general and understand use cases for things like this a bit more. My biggest concern would be merging a PR like this and/or #2651, then realizing later on that there should be a better name, different data format, etc.

So the more details you can provide -especially a real world repro- the better. Once #5038 is in having an issue filed with the info asked for there would be great. I can do that if going through the paperwork yet again isn't on your end-of-year wishlist. 🙂

Otherwise, some number of weeks (or perhaps months 😞) as we get acclimated.

Copy link

CLA Not Signed

@coveralls
Copy link

Coverage Status

coverage: 94.334% (-0.003%) from 94.337%
when pulling 76b36a1 on ashleyfrieze:json-reporter-assign-object
into eca4fec on mochajs:master.

@ashleyfrieze
Copy link
Author

@JoshuaKGoldberg - the scenario I was trying to cover was to detect the outcome of a Mocha test that was being run by something other than a usual npm or IDE test runner.

In particular, I was running mocha inside a Java test. The sut code in question was the embedded javascript being served by the Java application server as part of its SSR. The project, though a Java project, essentially had some JavaScript in it, and wrapping a mocha test within a JUnit test runner required the mocha tests to be executed, but the test runner to be able to acquire the results at the end.

Using a reporter, which put the results into a target object, seemed a neat way to do this, as achieved by the original PR.

I did a remix in this PR, which I also provided a test pack for.

It's a nice to have feature, and pretty innocuous. I worked around it somehow in my project, but could imagine coming back to this and wanting it in future Java SSR situations, or in situations where some mocha tests are being run by a different test framework as part of a transition plan.

@JoshuaKGoldberg JoshuaKGoldberg removed the stale this has been inactive for a while... label Mar 4, 2024
@JoshuaKGoldberg JoshuaKGoldberg marked this pull request as draft March 4, 2024 13:59
@JoshuaKGoldberg
Copy link
Member

JoshuaKGoldberg commented Mar 4, 2024

Converting to draft pending triage of #5111.

Btw, thanks for the added context - it's helpful!

@JoshuaKGoldberg JoshuaKGoldberg added the status: blocked Waiting for something else to be resolved label Mar 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: blocked Waiting for something else to be resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants