Support dashed args #7497

hulkish · 2018-12-11T10:59:42Z

Summary

Test plan

Added tests & ran:

yarn build && yarn jest

hulkish · 2018-12-11T11:03:11Z

@SimenB I imagine docs will need to be updated to reflect support for dashed args?

SimenB · 2018-12-11T11:57:13Z

Can we either

explore if yargs has an option for "merge options" which merges { 'foo-bar': true, fooBar: true } into { fooBar: true }
just use https://github.com/sindresorhus/camelcase-keys on argv returned from yargs

I imagine docs will need to be updated to reflect support for dashed args?

Yeah, a note saying both ways of defining the options are supported would be nice 🙂

Mind updating the changelog as well?

hulkish · 2018-12-11T12:15:35Z

@SimenB

explore if yargs has an option for "merge options" which merges { 'foo-bar': true, fooBar: true } into { fooBar: true }
Doesn't appear to be so: https://github.com/laggingreflex/yargs-parser/blob/db92d552ec8869445209fc6dd54ab51be73e8f5f/index.js#L97

SimenB · 2018-12-11T12:16:44Z

Merging on our side is fine, then 🙂 I assume the values have been normalized, so it's not a lossy transformation

hulkish · 2018-12-11T12:17:57Z

@SimenB

just use sindresorhus/camelcase-keys on argv returned from yargs

The problem i see with this - is that it will convert dashed to camel. Then, it will report as if you attempted the camelcased arg.

hulkish · 2018-12-11T12:18:41Z

@SimenB ok, updated docs & changelog

rickhanlonii · 2018-12-11T17:53:20Z

Is the change here that you could pass args as dashed instead of camel case? Is there a reason we shouldn't enforce the current camelCase style?

SimenB · 2018-12-11T18:04:33Z

Standard cli is dashes, not camel. Jest is the only cli I know of that does camelCase

hulkish · 2018-12-12T12:55:32Z

@SimenB @rickhanlonii I noticed this is failing the node@6 ci build. Should it be passing, in order to allow merge?

Btw, (unrelated to above): latest release 24.0.0-alpha.7 is fubar - missing build dirs in jest packages. Ex:

$ mkdir foo && cd foo && yarn init --yes
$ yarn add jest-cli@24.0.0-alpha.7
$ ls node_modules/jest-cli/
> bin/  node_modules/  LICENSE  README.md  package.json
$ cat node_modules/jest-cli/package.json | jq -r .main
> build/jest.js

SimenB · 2018-12-12T13:00:28Z

Node 6 has to pass, yes 🙂 However, the failure looked transient, so I restarted the whole build. 🤞

hulkish · 2018-12-12T13:37:02Z

@SimenB found out why it was failing: Error: You can only use --detectOpenHandles on Node 8 and newer. - which is from my test. So, easy fix by swapping it out with a diff dashed arg... this should be good to go now

codecov-io · 2018-12-12T13:56:46Z

Codecov Report

Merging #7497 into master will decrease coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #7497      +/-   ##
==========================================
- Coverage   67.46%   67.45%   -0.01%     
==========================================
  Files         247      247              
  Lines        9516     9514       -2     
  Branches        5        6       +1     
==========================================
- Hits         6420     6418       -2     
  Misses       3094     3094              
  Partials        2        2

Impacted Files	Coverage Δ
packages/jest-validate/src/validateCLIOptions.js	`78.94% <100%> (+1.8%)`	⬆️
packages/jest-config/src/getMaxWorkers.js	`100% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df78255...3f45e11. Read the comment docs.

rickhanlonii

LGTM, just a couple comments on documentation

rickhanlonii · 2018-12-12T16:57:44Z

CHANGELOG.md

@@ -38,6 +38,7 @@

 ### Fixes

+- `[jest-cli]` Support dashed args ([#7497](https://github.com/facebook/jest/pull/7497))


Can you move this below the breaking changes, preferably at the bottom of this section?

rickhanlonii · 2018-12-12T16:58:12Z

docs/CLI.md

@@ -81,6 +81,21 @@ you can use:
 npm test -- -u -t="ColorPicker"
 ```

+## Camelcase & dashed args support
+
+Jest supports both camelcase and dashed arg formats. Which means the following examples will have equal result:


Suggested change

Jest supports both camelcase and dashed arg formats. Which means the following examples will have equal result:

Jest supports both camelcase and dashed arg formats. The following examples will have equal result:

rickhanlonii · 2018-12-12T16:58:33Z

docs/CLI.md

+jest --collectCoverage
+```
+
+They can also be mixed:


Suggested change

They can also be mixed:

Arguments can also be mixed:

hulkish · 2018-12-12T21:04:15Z

@rickhanlonii ok all set, I think.

SimenB · 2018-12-13T08:32:01Z

CI still failing

SimenB

Just to verify, the object passed around as globalObject after jest-config has done its thing has just the camelCase keys, right?

SimenB · 2018-12-13T08:32:36Z

packages/jest-validate/src/validateCLIOptions.js

-    arg => !allowedOptions.has(arg),
-  );
+  const unrecognizedOptions = Object.keys(argv).filter(arg => {
+    const camelCased = arg.replace(/-([^-])/g, (a, b) => b.toUpperCase());


Maybe add camelize dep?

hulkish · 2018-12-13T11:53:03Z

@SimenB

Just to verify, the object passed around as globalObject after jest-config has done its thing has just the camelCase keys, right?

Hmm so good question - because, this is already an issue in master + this pr. I'll include that fix here:


const buildArgv = (maybeArgv: ?Argv, project: ?Path) => {
  const rawArgv: Argv | string[] = maybeArgv || process.argv.slice(2);
  const argv: Argv = yargs(rawArgv)
    .usage(args.usage)
    .alias('help', 'h')
    .options(args.options)
    .epilogue(args.docs)
    .check(args.check).argv;

  validateCLIOptions(
    argv,
    Object.assign({}, args.options, {deprecationEntries}),
    Array.isArray(rawArgv)
      ? rawArgv.map(rawArgv => rawArgv.replace(/^--?/, ''))
      : Object.keys(rawArgv),
  );

  return argv;
};

aka...remove the dashed entries of argv on the return.

hulkish · 2018-12-13T13:16:13Z

@SimenB k, using camelcase, filtering returned argv result & added test for it

hulkish · 2018-12-13T13:53:24Z

@SimenB ~~hmm can u restart these failed ci runs? looks network related~~

nvm, fixed

hulkish · 2018-12-13T14:26:34Z

@SimenB k, ci passing now

rickhanlonii · 2018-12-13T18:16:14Z

Thanks @hulkish and congrats on your first Jest PR!

* master: (24 commits) Add `jest.isolateModules` for scoped module initialization (jestjs#6701) Migrate to Babel 7 (jestjs#7016) docs: changed "Great Scott!" link (jestjs#7524) Use reduce instead of filter+map in dependency_resolver (jestjs#7522) Update Configuration.md (jestjs#7455) Support dashed args (jestjs#7497) Allow % based configuration of max workers (jestjs#7494) chore: Standardize filenames: jest-runner pkg (jestjs#7464) allow `bail` setting to control when to bail out of a failing test run (jestjs#7335) Add issue template labels (jestjs#7470) chore: standardize filenames in e2e/babel-plugin-jest-hoist (jestjs#7467) Add node worker-thread support to jest-worker (jestjs#7408) Add `testPathIgnorePatterns` to CLI documentation (jestjs#7440) pretty-format: Omit non-enumerable symbol properties (jestjs#7448) Add Jest Architecture overview to docs. (jestjs#7449) chore: run appveyor tests on node 10 chore: fix failures e2e test for node 8 (jestjs#7446) chore: update docusaurus to v1.6.0 (jestjs#7445) Enhancement/expect-to-be-close-to-with-infinity (jestjs#7444) Update CHANGELOG formatting (jestjs#7429) ...

* added dashed arg support to fix jestjs#7424 * update docs & changelog * fix dashed arg support tests * move changelog item to correct location, small edit to docs * use camelcase, filter dashed args on returns config

github-actions · 2021-05-12T05:14:10Z

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

added dashed arg support to fix jestjs#7424

7c3e1ef

facebook-github-bot added the cla signed label Dec 11, 2018

hulkish mentioned this pull request Dec 11, 2018

Unrecognized cli patterns for jest@beta #7424

Closed

update docs & changelog

a9999f6

SimenB requested a review from rickhanlonii December 11, 2018 12:18

hulkish changed the title ~~support dashed args~~ Support dashed args Dec 11, 2018

fix dashed arg support tests

ec9ba73

rickhanlonii approved these changes Dec 12, 2018

View reviewed changes

SimenB reviewed Dec 13, 2018

View reviewed changes

move changelog item to correct location, small edit to docs

3f45e11

hulkish force-pushed the support-dashed-args branch from 222166b to 3f45e11 Compare December 13, 2018 11:44

hulkish force-pushed the support-dashed-args branch 2 times, most recently from 0d1a477 to 90ce0fd Compare December 13, 2018 13:22

hulkish force-pushed the support-dashed-args branch from 90ce0fd to 096d750 Compare December 13, 2018 14:12

use camelcase, filter dashed args on returns config

bad4999

hulkish force-pushed the support-dashed-args branch from 096d750 to bad4999 Compare December 13, 2018 14:17

rickhanlonii merged commit 700e0da into jestjs:master Dec 13, 2018

bidoubiwa mentioned this pull request Dec 10, 2020

Add resolution to fix security altert meilisearch/docs-searchbar.js#181

Closed

github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support dashed args #7497

Support dashed args #7497

hulkish commented Dec 11, 2018

hulkish commented Dec 11, 2018

SimenB commented Dec 11, 2018 •

edited

hulkish commented Dec 11, 2018

SimenB commented Dec 11, 2018

hulkish commented Dec 11, 2018

hulkish commented Dec 11, 2018

rickhanlonii commented Dec 11, 2018

SimenB commented Dec 11, 2018

hulkish commented Dec 12, 2018

SimenB commented Dec 12, 2018

hulkish commented Dec 12, 2018

codecov-io commented Dec 12, 2018 •

edited

rickhanlonii left a comment

rickhanlonii Dec 12, 2018

rickhanlonii Dec 12, 2018

rickhanlonii Dec 12, 2018

hulkish commented Dec 12, 2018

SimenB commented Dec 13, 2018

SimenB left a comment

SimenB Dec 13, 2018

hulkish commented Dec 13, 2018

hulkish commented Dec 13, 2018

hulkish commented Dec 13, 2018 •

edited

hulkish commented Dec 13, 2018

rickhanlonii commented Dec 13, 2018

github-actions bot commented May 12, 2021

		@@ -38,6 +38,7 @@

		### Fixes

		- `[jest-cli]` Support dashed args ([#7497](https://github.com/facebook/jest/pull/7497))

	Jest supports both camelcase and dashed arg formats. Which means the following examples will have equal result:
	Jest supports both camelcase and dashed arg formats. The following examples will have equal result:

Support dashed args #7497

Support dashed args #7497

Conversation

hulkish commented Dec 11, 2018

Summary

Test plan

hulkish commented Dec 11, 2018

SimenB commented Dec 11, 2018 • edited

hulkish commented Dec 11, 2018

SimenB commented Dec 11, 2018

hulkish commented Dec 11, 2018

hulkish commented Dec 11, 2018

rickhanlonii commented Dec 11, 2018

SimenB commented Dec 11, 2018

hulkish commented Dec 12, 2018

SimenB commented Dec 12, 2018

hulkish commented Dec 12, 2018

codecov-io commented Dec 12, 2018 • edited

Codecov Report

rickhanlonii left a comment

Choose a reason for hiding this comment

rickhanlonii Dec 12, 2018

Choose a reason for hiding this comment

rickhanlonii Dec 12, 2018

Choose a reason for hiding this comment

rickhanlonii Dec 12, 2018

Choose a reason for hiding this comment

hulkish commented Dec 12, 2018

SimenB commented Dec 13, 2018

SimenB left a comment

Choose a reason for hiding this comment

SimenB Dec 13, 2018

Choose a reason for hiding this comment

hulkish commented Dec 13, 2018

hulkish commented Dec 13, 2018

hulkish commented Dec 13, 2018 • edited

hulkish commented Dec 13, 2018

rickhanlonii commented Dec 13, 2018

github-actions bot commented May 12, 2021

SimenB commented Dec 11, 2018 •

edited

codecov-io commented Dec 12, 2018 •

edited

hulkish commented Dec 13, 2018 •

edited