Update to a more recent version of mocha

[albertyw]

I keep getting security warnings from github and npm when depending on this package. On both the latest release and master, I see

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ minimist                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.2.1 <1.0.0 || >=1.2.3                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ 80897cf53105dcd799d9cd1b63b9d548afb11e934eaeb9327be2ebafa8b… │
│               │ [dev]                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ 80897cf53105dcd799d9cd1b63b9d548afb11e934eaeb9327be2ebafa8b… │
│               │ > mocaccino > mocha > mkdirp > minimist                      │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1179                            │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ Low           │ Prototype Pollution                                          │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ minimist                                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in    │ >=0.2.1 <1.0.0 || >=1.2.3                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ 80897cf53105dcd799d9cd1b63b9d548afb11e934eaeb9327be2ebafa8b… │
│               │ [dev]                                                        │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ 80897cf53105dcd799d9cd1b63b9d548afb11e934eaeb9327be2ebafa8b… │
│               │ > mocha > mkdirp > minimist                                  │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://npmjs.com/advisories/1179                            │
└───────────────┴──────────────────────────────────────────────────────────────┘

which I suspect is both mochify and mocaccino depending on mocha v5 (the current major release of mocha is v7). I'd submit pull requests for both mochify and mocaccino to update to the latest versions of mocha but I think mocaccino's dependency on phantomjs is causing TypeError: undefined is not a function (evaluating 'Array.from(arguments)'). That error I suspect is from phantomjs using an old javascript engine. Therefore, updating mocaccino is dependent on switching its tests to chromium (or using a polyfill to make phantomjs support Array.from) and updating mochify is dependent on updating mocaccino (mochify sees the same exact error when updating mocha).

I recorded the mocaccino phantomjs issue in mantoni/mocaccino.js#29.

[mantoni]

Yes, this is an issue and I would like to upgrade to a newer Mocha version too. I'm more than happy to merge any PRs that upgrade Mocha in the current implementation.

However, be warned that I've started a complete rewrite of Mochify with a different achitecture. It might take some time to complete, so it's great if you can help with upgrading Mocha in the current implementation. Just don't put too much effort into it 😉. Maybe you can try an Array.from polyfill, just for the tests?

[m90]

I looked into upgrading mocaccino to Mocha 8 over at this branch: https://github.com/mantoni/mocaccino.js/tree/mocha-update

There were some changes in the mocha API to consider and output format changed slightly. In the end everything seems to work well in Node.

---

Apart from that two reporter tests fail in Phantom.js for reasons I do not understand:

uses-reporter

I have no idea at all. Polyfilling Array.from as suggested did not make a difference.

TypeError: Type error
    at http://localhost:43381/js/bundle:27703
    at http://localhost:43381/js/bundle:27703
    at http://localhost:43381/js/bundle:3309
    at http://localhost:43381/js/bundle:3394
    at http://localhost:43381/js/bundle:25712
    at http://localhost:43381/js/bundle:25735
    at http://localhost:43381/js/bundle:31535
TypeError: Type error
    at http://localhost:43381/js/bundle:27719
    at http://localhost:43381/js/bundle:27719
    at http://localhost:43381/js/bundle:3331
    at http://localhost:43381/js/bundle:3402
    at http://localhost:43381/js/bundle:25100
    at http://localhost:43381/js/bundle:25645
    at http://localhost:43381/js/bundle:0
    at http://localhost:43381/js/bundle:31508

uses-custom-reporter

Looks like this reporter might not be compatible with Mocha 8 anymore. I tried using the next best npm package I could find but in this case it failed as the code was using const and the likes which Phantom.js does not know about.

TypeError: invalid reporter 'mocha-jenkins-reporter'
    at http://localhost:37701/js/bundle:37969
    at http://localhost:37701/js/bundle:39445
    at http://localhost:37701/js/bundle:1
    at http://localhost:37701/js/bundle:1
undefined: 'mocha-jenkins-reporter' reporter blew up with error:
commonjsRequire@http://localhost:37701/js/bundle:9333:93
reporter@http://localhost:37701/js/bundle:37953:38
http://localhost:37701/js/bundle:39445:15
o@http://localhost:37701/js/bundle:1:277
r@http://localhost:37701/js/bundle:1:440
global code@http://localhost:37701/js/bundle:1:468

Considering Mochify does not use Phantom.js anyways anymore, would it be a thing to drop the support (mantoni/mocaccino.js#29) over at mocaccino so that this does not keep us locked to old Mocha versions in mochify?

[mantoni]

Thank you for looking into this. We can drop PhantomJS, that's fine with me.

[m90]

Instead of just deleting all the browser tests in mocaccino I made the "mistake" to port them to some mini-Mochify like setup that runs the tests in a browser and found out about the following:

Mocha 8+ uses a Rollup-bundled browser version that seems to be incompatible with how mocaccino imports custom reporters, making it throw

"Error: Dynamic requires are not currently supported by @rollup/plugin-commonjs
    at commonjsRequire (http://127.0.0.1:7890/:9334:9)
    at Mocha.reporter (http://127.0.0.1:7890/:37954:23)
    at Object.require.40 (http://127.0.0.1:7890/:39446:7)
    at o (http://127.0.0.1:7890/:2:273)
    at r (http://127.0.0.1:7890/:2:439)
    at http://127.0.0.1:7890/:2:468"

which smells like an issue in Mocha itself (and cannot be fixed in mocaccinio as is - but I might be wrong about this) as they do prescribe a dynamic import here: https://github.com/mochajs/mocha/blob/5064c282d13259925af05845026686bbe435d763/lib/mocha.js#L340-L342

I'll dig into this further when I find the time.

[mantoni]

Ah, I thought you are aware of that issue. I filed a PR to fix this in Mocha, but they don't seem to be interested in supporting browserify anymore.

mochajs/mocha#4121

Therefore my future strategy is to build Mochify differently from the ground up, but I didn't find the time doing this, apart from some experiments.

[m90]

So I managed to work around the dynamic require issue by requiring the reporter in the setup script mantoni/mocaccino.js@df8ffea

There is one more issue remaining though that I don't fully understand yet: Some built-in reporters (list for example) now start printing format strings and their args instead of interpolating these. For example the list reporter test now prints:

  ✓ %s: %dms fixture passes 0
  %d passing (%s) 1 4ms

when previously we'd see:

  ✓ fixture passes: 0ms
  1 passing (4ms)

This seems to have been introduced with Mocha 6 (it fails for 6, 7 and 8), although I cannot really find anything in the changelog here https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#600--2019-02-18 that sound like it could be causing this?

Is this possibly related to brout?

---

Failing build here: https://travis-ci.org/github/mantoni/mocaccino.js/jobs/771339993

[m90]

So the log formatting change has been introduced in Mocha 6.2.0 to be exact it seems: https://github.com/mochajs/mocha/blob/master/CHANGELOG.md#620--2019-07-18

I suspect it's this change: mochajs/mocha#3725 but I don't fully understand where this breaks right now. If you have any hint @mantoni that'd be much appreciated.

[m90]

It seems brout now needs to be required before mocha, but if you do it like this it works as expected: mantoni/mocaccino.js@9b4e1b2

I'll open a PR for this in the mocaccino repo tomorrow so we can have a closer look at the mess over there 🌔

[m90]

@mantoni Just so we don't duplicate efforts, I started the Mocha 8 update for Mochify itself here https://github.com/mantoni/mochify.js/tree/mocha-8

There's some strange logging of deprecation notices going on in Chromium, but I think I should be able to look into these soon.