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

Support require and reporter-options in mocharc #4142

Closed
brettz9 opened this issue Jan 6, 2020 · 12 comments
Closed

Support require and reporter-options in mocharc #4142

brettz9 opened this issue Jan 6, 2020 · 12 comments

Comments

@brettz9
Copy link

brettz9 commented Jan 6, 2020

Is your feature request related to a problem or a nice-to-have?? Please describe.

I am not able to get reporter-options (or reporterOptions) working with either _mocha or mocha when the config is within mocha on package.json or in .mochrarc.js

require is also not working either as an array or set of strings.

But other properties are working.

Describe the solution you'd like

I'd like it to be possible to add reporterOptions and require within mocharc.js.

Describe alternatives you've considered

It seems unnecessarily clunky to add as flags to the CLI, especially when I've already added an RC file.

Additional context

@brettz9 brettz9 added the type: feature enhancement proposal label Jan 6, 2020
@boneskull boneskull added unconfirmed-bug and removed type: feature enhancement proposal labels Jan 7, 2020
@boneskull
Copy link
Contributor

These should be working. Can you provide a sample repo or configuration?

@boneskull boneskull added the status: waiting for author waiting on response from OP - more information needed label Jan 7, 2020
@brettz9
Copy link
Author

brettz9 commented Jan 7, 2020

On master of https://github.com/brettz9/intl-dom, I'm not using it currently, but on this branch it is failing: https://github.com/brettz9/intl-dom/tree/mocha-failing .

The reporter options in master point mocha-multi-reporters to the right configFile, i.e., to mocha-multi-reporters.json which triggers mocha-badge-generator to run producing output at badges/test-badges.svg. But in the branch, the reporter-options are not passed on.

If this could be an issue with mocha-multi-reporters I seem to remember replicating with another config without it, and can look to reproduce as needed.

@kellyselden
Copy link

This is also happening to me. https://github.com/CrowdStrike/faltest/blob/58ef39600c/packages/mocha/src/index.js#L55

My junit is printing to console instead of taking reporter options "output=dist/test_output.xml".

The repro is to check out that repo, use commit 7857ec083be2867eec901d737954c3cc7d56201c (right before mocha 7). Then yarn install and run yarn start examples/runner-only/tests/* --reporter xunit --reporter-options output=dist/test_output.xml. Notice the dist file.

Then checkout d03d8fdd7e5467466f35c23f2f3936320e3f3f3e (mocha 7 commit), (remove old dist file), yarn install and run the command again. Notice it printing to console instead of file.

@kellyselden
Copy link

After reverting mocha 7 and adding a regression test, this is now the failing PR if it helps. CrowdStrike/faltest#219

@juergba
Copy link
Contributor

juergba commented Jan 11, 2020

@kellyselden This has been a bug in v6. The Mocha constructor was expecting opts.reporterOption, but processed reporterOptions. see API docs.

kellyselden pushed a commit to CrowdStrike/faltest that referenced this issue Jan 11, 2020
instead of intuitive `reporterOptions`
mochajs/mocha#4142 (comment)
@kellyselden
Copy link

Thank you @juergba. I fixed it by changing to reporterOption. I missed that in the changelog. Sorry @brettz9 for unintentionally hijacking your issue.

kellyselden pushed a commit to CrowdStrike/faltest that referenced this issue Jan 13, 2020
instead of intuitive `reporterOptions`
mochajs/mocha#4142 (comment)
holm pushed a commit to peakon/mocha that referenced this issue Jan 14, 2020
In mochajs#4004 there was a change to use the documented `reporterOption` in favour of the setting that had always been used in practice called `reporterOptions`. This broke a lot of configurations that used the `reporterOptions`, which was the only way it every worked AFAIK.

This changes the documentation to specify `reporterOptions` instead and ensure that any one that has switched to `reporterOption` after upgrade, still works.
holm pushed a commit to peakon/mocha that referenced this issue Jan 19, 2020
In mochajs#4004 there was a change to use the documented `reporterOption` in favour of the setting that had always been used in practice called `reporterOptions`. This broke a lot of configurations that used the `reporterOptions`, which was the only way it every worked AFAIK.

This changes the documentation to specify `reporterOptions` instead and ensure that any one that has switched to `reporterOption` after upgrade, still works.
holm pushed a commit to peakon/mocha that referenced this issue Jan 19, 2020
In mochajs#4004 there was a change to use the documented `reporterOption` in favour of the setting that had always been used in practice called `reporterOptions`. This broke a lot of configurations that used the `reporterOptions`, which was the only way it every worked AFAIK.

This changes the documentation to specify `reporterOptions` instead and ensure that any one that has switched to `reporterOption` after upgrade, still works.
holm pushed a commit to peakon/mocha that referenced this issue Jan 20, 2020
In mochajs#4004 there was a change to use the documented `reporterOption` in favour of the setting that had always been used in practice called `reporterOptions`. This broke a lot of configurations that used the `reporterOptions`, which was the only way it every worked AFAIK.

This changes the documentation to specify `reporterOptions` instead and ensure that any one that has switched to `reporterOption` after upgrade, still works.
@juergba
Copy link
Contributor

juergba commented Apr 10, 2020

@brettz9 reporter-option in .mocharc.js is an array. Please try:

`reporter-option`: [
    'configFile=mocha-multi-reporters.json'
]

@brettz9
Copy link
Author

brettz9 commented Apr 10, 2020

Excellent, thank you, @juergba, I see this is working now. However, for require, it still does not work:

  require: [
    'esm', 'chai/register-expect', 'test/bootstrap/node.js'
  ]

@brettz9
Copy link
Author

brettz9 commented Apr 10, 2020

Hmmm... Ok, I was left with:

_mocha --require esm --require chai/register-expect --require test/bootstrap/node.js

which after removing the requires, I just had:

_mocha

...which was complaining about an unexpected idenifier, but it is ok with "mocha".

@brettz9
Copy link
Author

brettz9 commented Apr 10, 2020

So I guess this is resolved as far as I am concerned (though I'd be interesting in knowing why _mocha is different).

@juergba
Copy link
Contributor

juergba commented Apr 10, 2020

mocha checks for Node flags and spawns a child-process if found any. --require esm is handled as Node flag, the rest of the require's are passed to Mocha.

_mocha is for backwards compability only and does not care for Node flags. It was used also for debugging, in times when debugging a child-process was difficult.

I would like to close this PR.

@brettz9 brettz9 closed this as completed Apr 10, 2020
@juergba juergba removed the status: waiting for author waiting on response from OP - more information needed label Apr 10, 2020
@ShazotiHashimoto
Copy link

ShazotiHashimoto commented Aug 28, 2023

@brettz9 reporter-option in .mocharc.js is an array. Please try:

`reporter-option`: [
    'configFile=mocha-multi-reporters.json'
]

THis worked for me

In case of mocharc.json use

  "reporterOptions": [
    "configFile=./reporters.json"
  ]

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants