-
Notifications
You must be signed in to change notification settings - Fork 57
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
Update to a more recent version of mocha #213
Comments
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 |
I looked into upgrading 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
|
Thank you for looking into this. We can drop PhantomJS, that's fine with me. |
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
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. |
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. 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. |
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 (
when previously we'd see:
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 Failing build here: https://travis-ci.org/github/mantoni/mocaccino.js/jobs/771339993 |
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. |
It seems I'll open a PR for this in the mocaccino repo tomorrow so we can have a closer look at the mess over there 🌔 |
@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. |
I keep getting security warnings from github and npm when depending on this package. On both the latest release and master, I see
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 supportArray.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.
The text was updated successfully, but these errors were encountered: