feat: randomization support [WIP REFERENCE - DO NOT MERGE] #2301

boneskull · 2016-06-10T01:12:18Z

Edit (January 2024, @JoshuaKGoldberg): This PR is just here as a reference to support #902. If you want to see this feature land, send a PR yourself. See #902 (comment). Thanks! ✨

Adds randomization support via --random CLI flag or random option in browser
Random seed is created if not supplied for deterministic test runs

QUESTIONS:

Automatic Seed Creation

How should we handle automatic seed creation?

Issues:

Some reporters (XUnit in particular) are extremely fussy about output.
If we were to simply echo the automatically created seed to the console, problems would happen

Suggestions:

In the browser, this can simply be displayed.
Output a .mocha-seed.txt file or something. This file would not be read from; only written to.
Disable automatic seed creation; require a seed for operation. Trivially generate a seed via Mocha.seed() in the browser, or --generate-seed on the CLI.

Update, Nov 20 2016:

"Specification"

"Automatic" seed generation is not (yet?) available, due to concerns about output and reporter support / conflicts.
A seed is required for randomization.
On the command line:
- Mocha can generate a seed via mocha --generate-seed. A random seed will be printed to the console.
- --random <seed> will enable randomization.
In the browser:
- Mocha can generate a seed via mocha.seed().
- Enable randomization thru mocha.options({random: mocha.seed()}) before calling mocha.run().
For Mocha's purposes, a seed is a positive integer which is less than or equal to the value of Number.MAX_SAFE_INTEGER, which is typically 9007199254740991.
Acceptable formats for seeds are numbers (decimal, octal, hexadecimal) or strings that will convert via parseInt().
Mocha will use a hexadecimal representation for display purposes.
When randomizing tests, they are only randomized within the context of a suite. In other words, suites will still run in order, but the tests within them will not.

TODO

boneskull · 2016-06-10T01:13:49Z

cc @mochajs/mocha

boneskull · 2016-06-10T01:17:26Z

cc @TimothyGu

ScottFreeCode · 2016-06-13T16:06:06Z

lib/mocha.js

@@ -12,6 +12,8 @@ var escapeRe = require('escape-string-regexp');
 var path = require('path');
 var reporters = require('./reporters');
 var utils = require('./utils');
+var Random = require('random-js');
+var MAX_SAFE_INTEGER = 0x1fffffffffffff;



Where is this number coming from?

I'd rather get the number from the JS API or, if not available, an npm module.

From https://www.npmjs.com/package/random-js#distributions

Random.integer(min, max)(engine): Produce an integer within the inclusive range [min, max]. min can be at its minimum -9007199254740992 (-2 ** 53). max can be at its maximum 9007199254740992 (2 ** 53).

Number.MAX_SAFE_INTEGER is the same value.

EDIT: Number.MAX_SAFE_INTEGER is only available in ES2015 and later.

Yes, sorry. 0x1fffffffffffff is the value of Number.MAX_SAFE_INTEGER if Number.MAX_SAFE_INTEGER is implemented. But we're not guaranteed that, of course, and I didn't want to pull in some polyfill. I'll add a comment.

@dasilvacontin Would prefer not to npm install --save max-safe-integer (don't know if that exists; probably does) if I can help it.

dasilvacontin · 2016-06-13T16:30:18Z

Thanks for putting time into this one peeps!

boneskull · 2016-06-27T06:47:41Z

@ScottFreeCode @TimothyGu Thanks for looking this over. I'll implement the suggested changes

boneskull · 2016-06-27T06:49:02Z

I don't think my questions in the description were answered though.

If I'm deciding, I'll go with --generate-seed so I don't have to deal with the file system.

TimothyGu · 2016-06-27T07:28:06Z

I'm not familiar with the current situation of browser testing. How do reporters interact with browsers? Can I use, say, the XUnit reporter with the browser? If yes, will the XUnit output be made to the browser console?

boneskull · 2016-06-27T09:03:02Z

so this is how it works on the CLI:

# generate the seed first
$ mocha --generate-seed 
0x10FFFA5D98AE6F

# run tests with the seed
$ mocha --random 0x10FFFA5D98AE6F test/acceptance/random.js --reporter spec


  random option
    seed
      ✓ should be a valid seed
      ✓ should return a finite number
      ✓ should return a hex string
    when Mocha is instantiated with "random" option
      and that option is "false"
        ✓ should not populate the "randomConfig" option
      and that option is undefined
        ✓ should not populate the "randomConfig" option
      and that option is not an integer or convertable string
        ✓ should throw
      and that option is a valid seed
        randomConfig
          ✓ should define a function to shuffle tests in the "shuffleTests" prop
          ✓ should define a "seed" prop
          ✓ should be defined
          ✓ should define a hexadecimal representation in the "hex" prop


  10 passing (24ms)

For comparison, without randomization:

$ mocha test/acceptance/random.js  --reporter spec


  random option
    seed
      ✓ should return a hex string
      ✓ should be a valid seed
      ✓ should return a finite number
    when Mocha is instantiated with "random" option
      and that option is "false"
        ✓ should not populate the "randomConfig" option
      and that option is undefined
        ✓ should not populate the "randomConfig" option
      and that option is not an integer or convertable string
        ✓ should throw
      and that option is a valid seed
        randomConfig
          ✓ should be defined
          ✓ should define a "seed" prop
          ✓ should define a hexadecimal representation in the "hex" prop
          ✓ should define a function to shuffle tests in the "shuffleTests" prop


  10 passing (24ms)

boneskull · 2016-06-27T09:06:31Z

@TimothyGu Generally, if you're in a browser, your only available reporter is the HTML reporter.

If you're in a headless browser, then the HTML reporter is going to just dump HTML into your terminal, and you'll want one of the console-based reporters, like spec. I would also assume something with the complexity of nyan would not work in this context. Furthermore, I doubt a reporter like json-stream would work in a PhantomJS context, but who knows.

I don't use Mocha in a browser context without Karma to hold my hand, so I'm not the best source of information on this topic. @mantoni is much more qualified to elaborate on which reporters are supported, if he cares to.

boneskull · 2016-06-27T09:11:27Z

What's left to do:

Display active seed in HTML reporter
Write integration tests to assert deterministic, random order of tests within a suite context
Docs
Support MOCHA_SEED environment variable or something. This would ease usage in CI envs.

boneskull · 2016-06-27T09:15:33Z

note to self:

Karma setup would look something like:

var seed = require('mocha').seed();
console.log('Mocha seed: ' + seed);
config.set({
  frameworks: ['mocha'],
  client: {
    random: seed
  }
});

TimothyGu · 2016-06-27T09:17:45Z

lib/utils.js

+};
+
+/**
+ * Returns `true` if `value` is a "safe" 32-bit unsigned int.


Here and in various other places, it's a bit confusing to see uint32_t or integers in the range of [0, 2³² – 1] being called "safe," while what the function actually takes is integers in the range of [1, 2⁵³ – 1]. IMO, what we should allow is [0, 2⁵³ – 1] which can be described as "safe" unsigned 53-bit integer and matches with ECMAScript semantics. (In addition, we can optionally allow negative values as well.)

boneskull · 2016-06-27T09:19:57Z

SauceLabs is taking a big ol' dump trying to run our tests in IE8. Considering the test-windows/appveyor branch hasn't been merged, it's not specific to my changes or @TimothyGu's changes there. Curiously, the pr tests pass, but the push tests fail. Must have something to do with trying to run in a branch. At least that's something to go on for SauceLabs--they're waiting for my reply to a ticket about this.

mantoni · 2016-06-27T09:39:29Z

@TimothyGu @boneskull Pretty much all the reporters work in PhantomJS, too. The trick is to redirect console output and to provide a shim for process.stdout. I do this with brout and then set Mocha.process.stdout = process.stdout (see mocaccino browser setup). With this setup, even the nyan reporter output is redirected properly to the terminal.

ScottFreeCode · 2016-10-17T20:39:30Z

lib/utils.js

-    // (traditional)->  space/name     parameters    body     (lambda)-> parameters       body   multi-statement/single          keep body content
+    // (traditional)->  space/name     parameters    body     (lambda)->
+    // parameters       body   multi-statement/single          keep body
+    // content
    .replace(/^function(?:\s*|\s+[^(]*)\([^)]*\)\s*\{((?:.|\n)*?)\s*\}$|^\([^)]*\)\s*=>\s*(?:\{((?:.|\n)*?)\s*\}|((?:.|\n)*))$/, '$1$2$3');



The comment wrapping here basically breaks this documentation of the regex. Do we need to stop the linter from paying attention to that comment? Or should we break up the regex into parts (as strings) and compose them using new RegExp?

didn't notice this. I'll ensure it's left as-is

macalinao · 2016-11-20T20:36:46Z

woohoo finally

boneskull · 2016-11-20T21:11:04Z

So basically needs more tests. But I don't necessarily need to be the person to write them.

Robdel12 · 2017-03-27T16:17:21Z

Is there an updated description of what needs to happen to push this over the line? We would like to randomize our mocha builds to bring the flaky tests to the surface.

boneskull · 2017-03-27T19:05:19Z

not so much. new features are frozen until we get traction on immediate coverage needs. though I haven't had any time to pay attention to where we're at there

craigtaub · 2019-05-11T22:56:06Z

@boneskull this sounds good. Worth pursuing now coverage in better place? Or do you think given where codebase is now there's a better alternative approach here?

JoshuaKGoldberg · 2023-12-02T21:09:15Z

~~🤖 Closing out old PRs to keep the review queue small. Cheers!~~

Edit: per #902 (comment), re-opening and marking as a draft.

linux-foundation-easycla · 2024-01-21T06:15:08Z

✅ login: boneskull / name: Christopher Hiller (ff75d4b, c568bef, 62c6b31, a6f6e6e, 5ed7851, e01cf0b, 3ff40dd, 75b84c5, 2a4cba2, 847b697, 60fec82, 75fe69f)
❌ - login: @TimothyGu / name: Timothy Gu . The commit (2fe3fe4) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

fearphage · 2024-01-21T10:05:27Z

lib/utils.js

+  var intVal;
+  if (/^0[xX]/.test(value)) {
+    intVal = parseInt(value, 16);
+  } else if (/^0[^.]/.test(value)) {
+    intVal = parseInt(value, 8);
+  } else {
+    intVal = parseInt(value, 10);
+  }


It feels like this could be simplified a bit.

Suggested change

var intVal;

if (/^0[xX]/.test(value)) {

intVal = parseInt(value, 16);

} else if (/^0[^.]/.test(value)) {

intVal = parseInt(value, 8);

} else {

intVal = parseInt(value, 10);

}

var base = 10;

var intVal;

if (/^0[xX]/.test(value)) {

base = 16;

} else if (/^0[^.]/.test(value)) {

base = 8;

}

intVal = parseInt(value, base);

fearphage · 2024-01-21T10:11:42Z

lib/utils.js

+  return !isNaN(intVal) && parseFloat(intVal) === intVal
+    && exports.isSafeUnsignedInteger(intVal)
+    ? intVal
+    : NaN;


isSafeUnsignedInteger already checks for an integer so there's no need to double check here. Also if the value is NaN, you'll still hit the right checks without the additional NaN check.

Suggested change

return !isNaN(intVal) && parseFloat(intVal) === intVal

&& exports.isSafeUnsignedInteger(intVal)

? intVal

: NaN;

return exports.isSafeUnsignedInteger(intVal) ? intVal : NaN;

fearphage · 2024-01-21T10:21:36Z

lib/utils.js

+ * @returns {string} Hexadecimal representation thereof
+ */
+exports.toHex = function toHex(int) {
+  return '0x' + int.toString(16).toUpperCase();


This doesn't have any validation compared to other functions.toHex('foo') will happily return 0xFOO. Is that expected behavior?

JoshuaKGoldberg · 2024-01-21T16:31:25Z

@fearphage thanks for the reviews, but this PR is just here as a reference. It's got a whole bunch of merge conflicts. I edited the title & OP to make that more clear.

If you're interested in the work (great! 🙌) the right next step would be to send a separate PR.

Edit: Closing to keep our review queue small.

boneskull added type: feature enhancement proposal High priority labels Jun 10, 2016

boneskull added this to the 2.6.0 milestone Jun 10, 2016

ScottFreeCode reviewed Jun 13, 2016
View reviewed changes

boneskull force-pushed the randomization branch from 9051348 to d1d3190 Compare June 27, 2016 08:13

TimothyGu reviewed Jun 27, 2016
View reviewed changes

boneskull modified the milestones: v3.1.0, 2.6.0 Jul 2, 2016

boneskull force-pushed the randomization branch from 2975dc5 to f28da97 Compare September 18, 2016 19:31

boneskull modified the milestones: v3.2.0, v3.1.0 Sep 18, 2016

boneskull added 4 commits September 18, 2016 22:38

update CLI

2a4cba2

actually works now: fix shuffling

847b697

fix some merges

60fec82

rename random.js => random.spec.js

75fe69f

boneskull force-pushed the randomization branch from f28da97 to 75fe69f Compare September 19, 2016 05:39

ScottFreeCode reviewed Oct 17, 2016

View reviewed changes

boneskull modified the milestone: v3.2.0 Nov 24, 2016

drazisil removed the high-priority label Mar 30, 2017

boneskull force-pushed the master branch 2 times, most recently from 4547268 to 7613521 Compare May 4, 2018 16:47

JoshuaKGoldberg closed this Dec 2, 2023

JoshuaKGoldberg deleted the randomization branch December 2, 2023 21:09

JoshuaKGoldberg restored the randomization branch January 21, 2024 06:12

JoshuaKGoldberg mentioned this pull request Jan 21, 2024

🚀 Feature: --order option for random test order #902

Open

JoshuaKGoldberg reopened this Jan 21, 2024

JoshuaKGoldberg marked this pull request as draft January 21, 2024 06:15

JoshuaKGoldberg added the status: waiting for author waiting on response from OP - more information needed label Jan 21, 2024

JoshuaKGoldberg changed the title ~~randomization support [WIP DO NOT MERGE]~~ feat: randomization support [WIP DO NOT MERGE] Jan 21, 2024

fearphage suggested changes Jan 21, 2024

View reviewed changes

JoshuaKGoldberg changed the title ~~feat: randomization support [WIP DO NOT MERGE]~~ feat: randomization support [WIP REFERENCE - DO NOT MERGE] Jan 21, 2024

JoshuaKGoldberg closed this Feb 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: randomization support [WIP REFERENCE - DO NOT MERGE] #2301

feat: randomization support [WIP REFERENCE - DO NOT MERGE] #2301

boneskull commented Jun 10, 2016 •

edited by JoshuaKGoldberg

Loading

boneskull commented Jun 10, 2016

boneskull commented Jun 10, 2016

ScottFreeCode Jun 13, 2016

dasilvacontin Jun 13, 2016

TimothyGu Jun 14, 2016 •

edited

Loading

boneskull Jun 27, 2016

boneskull Jun 27, 2016

dasilvacontin commented Jun 13, 2016

boneskull commented Jun 27, 2016

boneskull commented Jun 27, 2016

TimothyGu commented Jun 27, 2016

boneskull commented Jun 27, 2016

boneskull commented Jun 27, 2016 •

edited

Loading

boneskull commented Jun 27, 2016

boneskull commented Jun 27, 2016

TimothyGu Jun 27, 2016

boneskull commented Jun 27, 2016

mantoni commented Jun 27, 2016

ScottFreeCode Oct 17, 2016

boneskull Oct 18, 2016

macalinao commented Nov 20, 2016

boneskull commented Nov 20, 2016

Robdel12 commented Mar 27, 2017

boneskull commented Mar 27, 2017

craigtaub commented May 11, 2019

JoshuaKGoldberg commented Dec 2, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jan 21, 2024

fearphage Jan 21, 2024

fearphage Jan 21, 2024

fearphage Jan 21, 2024

JoshuaKGoldberg commented Jan 21, 2024 •

edited

Loading

feat: randomization support [WIP REFERENCE - DO NOT MERGE] #2301

feat: randomization support [WIP REFERENCE - DO NOT MERGE] #2301

Conversation

boneskull commented Jun 10, 2016 • edited by JoshuaKGoldberg Loading

Automatic Seed Creation

Update, Nov 20 2016:

"Specification"

TODO

boneskull commented Jun 10, 2016

boneskull commented Jun 10, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TimothyGu Jun 14, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dasilvacontin commented Jun 13, 2016

boneskull commented Jun 27, 2016

boneskull commented Jun 27, 2016

TimothyGu commented Jun 27, 2016

boneskull commented Jun 27, 2016

boneskull commented Jun 27, 2016 • edited Loading

boneskull commented Jun 27, 2016

boneskull commented Jun 27, 2016

Choose a reason for hiding this comment

boneskull commented Jun 27, 2016

mantoni commented Jun 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

macalinao commented Nov 20, 2016

boneskull commented Nov 20, 2016

Robdel12 commented Mar 27, 2017

boneskull commented Mar 27, 2017

craigtaub commented May 11, 2019

JoshuaKGoldberg commented Dec 2, 2023 • edited Loading

linux-foundation-easycla bot commented Jan 21, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoshuaKGoldberg commented Jan 21, 2024 • edited Loading

boneskull commented Jun 10, 2016 •

edited by JoshuaKGoldberg

Loading

TimothyGu Jun 14, 2016 •

edited

Loading

boneskull commented Jun 27, 2016 •

edited

Loading

JoshuaKGoldberg commented Dec 2, 2023 •

edited

Loading

JoshuaKGoldberg commented Jan 21, 2024 •

edited

Loading