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

feat: allow to set the seed for --shuffle with --seed. #657

Merged
merged 1 commit into from Nov 7, 2016

Conversation

@rmehner
Copy link
Contributor

rmehner commented Nov 3, 2016

This is useful, when you run into an order dependent test failure. The
runner reports the used seed after the test run. If you want to maintain
the same test run order for the next run, you can re-use the seed and
you get the same order.

For example: lab test/unit --shuffle --seed 1234

Fixes #656.

There is one unrelated test failure that is already fixed in #655

@gr2m

This comment has been minimized.

Copy link

gr2m commented Nov 3, 2016

The runner reports the used seed after the test run

Only when run with --verbose, is that intended? Works for me 👍

For example: lab test/unit --shuffle --seed 1234

The seed option does not get accepted for me, I get: Unknown option: seed=0.16460420544342202

This change fixed it for me

diff --git a/lib/cli.js b/lib/cli.js
index b5f8155..bbc6080 100755
--- a/lib/cli.js
+++ b/lib/cli.js
@@ -22,9 +22,7 @@ const internals = {};
 internals.rcPath = FindRc('lab');
 internals.rc = internals.rcPath ? require(internals.rcPath) : {};

-
 exports.run = function () {
-
     const settings = internals.options();

     settings.coveragePath = Path.join(process.cwd(), settings['coverage-path'] || '');
@@ -339,6 +337,11 @@ internals.options = function () {
             type: 'boolean',
             description: 'version information',
             default: null
+        },
+        seed: {
+          type: 'string',
+          description: 'use this seed to randomize the order with `--shuffle`. This is useful to debug order dependent test failures',
+          default: null
         }
     };

@@ -391,7 +394,7 @@ internals.options = function () {
         'coverage-path', 'debug', 'dry', 'environment', 'flat', 'globals', 'grep',
         'lint', 'lint-errors-threshold', 'lint-fix', 'lint-options', 'lint-warnings-threshold',
         'linter', 'output', 'parallel', 'pattern', 'reporter', 'shuffle', 'silence',
-        'silent-skips', 'sourcemaps', 'threshold', 'timeout', 'transform', 'verbose'];
+        'silent-skips', 'sourcemaps', 'threshold', 'timeout', 'transform', 'verbose', 'seed'];
     for (let i = 0; i < keys.length; ++i) {
         if (argv.hasOwnProperty(keys[i]) && argv[keys[i]] !== undefined && argv[keys[i]] !== null) {
             options[keys[i]] = argv[keys[i]];

With that change it all works as advertised, this is awesome, thanks @rmehner 👏

@rmehner rmehner force-pushed the rmehner:bugfixes/656-order-by-seed branch from 4a6e1eb to 5ab6ffd Nov 3, 2016
@rmehner

This comment has been minimized.

Copy link
Contributor Author

rmehner commented Nov 3, 2016

Good catch @gr2m, fixed & force pushed :)

@rmehner rmehner force-pushed the rmehner:bugfixes/656-order-by-seed branch from 5ab6ffd to 3fc9a10 Nov 4, 2016
@rmehner

This comment has been minimized.

Copy link
Contributor Author

rmehner commented Nov 4, 2016

I've rebased this based on what's currently in master (since #655 was merged in the meanwhile). Tests are green now :)

@geek

This comment has been minimized.

Copy link
Member

geek commented Nov 5, 2016

@rmehner there are a couple of lint issues after the latest eslint update: https://travis-ci.org/hapijs/lab/jobs/173241128

This is useful, when you run into an order dependent test failure. The
runner reports the used seed after the test run. If you want to maintain
the same test run order for the next run, you can re-use the seed and
you get the same order.

For example: `lab test/unit --shuffle --seed 1234`

Fixes #656
@rmehner rmehner force-pushed the rmehner:bugfixes/656-order-by-seed branch from 3fc9a10 to 6e637d3 Nov 7, 2016
@rmehner

This comment has been minimized.

Copy link
Contributor Author

rmehner commented Nov 7, 2016

All rebased and fixed linting errors :)

@geek geek merged commit 7c18c69 into hapijs:master Nov 7, 2016
2 checks passed
2 checks passed
Node Security No known vulnerabilities found
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@rmehner rmehner added the feature label Nov 7, 2016
@rmehner rmehner added this to the 11.2.0 milestone Nov 7, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.