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

Export yargs options and check function for consumption by wrapper modules #4122

Closed
wants to merge 2 commits into from
Closed

Export yargs options and check function for consumption by wrapper modules #4122

wants to merge 2 commits into from

Conversation

Jack-Barry
Copy link

@Jack-Barry Jack-Barry commented Dec 6, 2019

Description of the Change

Exports the Yargs options and check function that mocha uses to allow wrapper modules to consume them. This is accomplished by assigning the options to an exportable object and the exporting the check function before they are used by mocha internally.

Alternate Designs

Without exporting the options and check, wrapper modules are stuck re-implementing the options and check themselves, which stinks 💩

Why should this be in core?

This would provide external modules what they need to avoid reinventing the wheel, and doesn't break or disrupt anything here.

Benefits

Allows us to implement a fix for this

Possible Drawbacks

I can't think of any.

Applicable issues

Aforementioned issue

  • Is this a breaking change (major release)?
  • Is it an enhancement (minor release)?
  • Is it a bug fix, or does it not impact production code (patch release)?

@jsf-clabot
Copy link

jsf-clabot commented Dec 6, 2019

CLA assistant check
All committers have signed the CLA.

@Jack-Barry
Copy link
Author

👋🏻 Just wanted to check in on this to see if it can be merged, would be a huge help to some changes needed on our end. Thanks!

@JamesMcMahon
Copy link

Would love to see this merged as it would be a great boon to https://github.com/sysgears/mochapack/.

@ghost
Copy link

ghost commented Apr 10, 2020

Wouldn't it make sense to get this feature into mocha?

@Jack-Barry
Copy link
Author

Incorporated latest changes from Mocha's master branch before resolving the conflict in current version of run.js.

@juergba
Copy link
Contributor

juergba commented Apr 15, 2020

@Jack-Barry I don't know wether this PR makes sense or not.
But this branch is a complete mess now after "Incorporated latest changes" with 44 commits and 99 files changed. Must be the corona virus, I would never merge this PR in that shape.
I recommend a clean rebase to master, then a forced push.

@coveralls
Copy link

coveralls commented Apr 15, 2020

Coverage Status

Coverage increased (+0.08%) to 92.874% when pulling 9d235b6 on Jack-Barry:export-cli-options into 54475eb on mochajs:master.

@Jack-Barry
Copy link
Author

@juergba Apologies for the messiness, I am still new to contributing to other peoples' projects and using Git features such as rebase. I have tried to clean up this PR as you suggested. I also added another change we'd need to make this a little better on our end to be able to use Mocha's check functionality.

If there is another way that Mocha can be initiated from a tool that wraps it in their own CLI aside from having to re-implement all of the Yargs options, I'm open to that alternative path. Otherwise, this is the best way we could come up with to allow our project to minimize re-implementing features already written in the Mocha codebase.

It allows us to parse Mocha-targeted args with a simpler implementation like this:

import { options, check } from 'mocha/lib/cli/run'
import { aliases, types } from 'mocha/lib/cli/run-option-metadata'

export const parseMochaArgs = (argv) =>
  yargs
    .options(options)
    .check(check(yargs))
    .alias(aliases)
    .array(types.array)
    .boolean(types.boolean)
    .string(types.string)
    .number(types.number)
    .parse(argv)

As opposed to re-implementing all of the options and the check functionality. In an ideal world, we would just have a parseMochaArgs function from Mocha that spits out the parsed arguments completely, but this seemed like a good start at least as it doesn't really require any further maintenance overhead for the Mocha team.

@Jack-Barry Jack-Barry changed the title Export yargs options for consumption by wrapper modules Export yargs options and check function for consumption by wrapper modules Apr 15, 2020
@github-actions
Copy link
Contributor

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 Mar 28, 2021
@github-actions github-actions bot closed this Apr 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale this has been inactive for a while...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants