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

Exposing filename in JSON, doc, and json-stream reporters #4219

Merged
merged 5 commits into from May 1, 2020
Merged

Exposing filename in JSON, doc, and json-stream reporters #4219

merged 5 commits into from May 1, 2020

Conversation

dfebs
Copy link
Contributor

@dfebs dfebs commented Mar 25, 2020

Description of the Change

This change makes the json, doc, and json-stream reporters output the test filenames in addition to their other attributes

Alternate Designs

No other designs were considered in the making of this pull request

Why should this be in core?

There currently is not a way to access filenames through reporters when running tests even though mocha has full access to it.

Benefits

People who want to see what file is associated with their test step will easily be able to see it in the json, doc, and json-stream reporters. This is especially useful for people with large code/test base where it may be difficult to find the test.

Possible Drawbacks

If we intend to encapsulate the test's filename, then this code change would be an issue

Applicable issues

This would be considered a minor release.

@jsf-clabot
Copy link

jsf-clabot commented Mar 25, 2020

CLA assistant check
All committers have signed the CLA.

@outsideris outsideris added the area: reporters involving a specific reporter label Apr 4, 2020
Copy link
Member

@outsideris outsideris left a comment

Choose a reason for hiding this comment

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

If we add filename to JSON reporter, we should consider to add it to other reporters like JSONStream reporter.

@coveralls
Copy link

coveralls commented Apr 23, 2020

Coverage Status

Coverage increased (+0.07%) to 93.133% when pulling bcb9c4c on Daniel0113:expose-test-filename into 6838eaf on mochajs:master.

@dfebs
Copy link
Contributor Author

dfebs commented Apr 24, 2020

@outsideris I have added filename exposure to the doc and json-stream reporters (and updated tests for all the reporters I touched). Unless there are other reporters we have in mind, this is ready for a second look 😄

@dfebs dfebs changed the title Exposing filename in JSON reporter Exposing filename in JSON, doc, and json-stream reporter Apr 24, 2020
@dfebs dfebs changed the title Exposing filename in JSON, doc, and json-stream reporter Exposing filename in JSON, doc, and json-stream reporters Apr 24, 2020
@boneskull boneskull added type: feature enhancement proposal semver-minor implementation requires increase of "minor" version number; "features" labels Apr 27, 2020
Copy link
Member

@boneskull boneskull left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I note that this will break #4245, but that's my problem.

If there's a case in which test.file is undefined (I am unsure if this is possible), these reporters will print undefined. Might be worth while to check for test.file, and if it's falsy, output (no filename) or something instead. Not a big deal, and this could be merged w/o it.

@dfebs
Copy link
Contributor Author

dfebs commented Apr 27, 2020

Given that I also can't think of a test that isn't located in a file, I'm good with merging as-is then 😄 . If anyone has strong opinions on the matter though I am open to that.

@boneskull boneskull merged commit 9c965c9 into mochajs:master May 1, 2020
@boneskull boneskull added this to the next milestone May 1, 2020
craigtaub pushed a commit that referenced this pull request May 21, 2020
* Exposing filename in JSON reporter

* Added filename to json-stream reporter

* updated tests and also fixed issue with json-stream reporter

* added filenames to doc reporter
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: reporters involving a specific reporter semver-minor implementation requires increase of "minor" version number; "features" type: feature enhancement proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants