Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Dependency update: Update mocha #2113

Closed
wants to merge 6 commits into from
Closed

Dependency update: Update mocha #2113

wants to merge 6 commits into from

Conversation

eggplantzzz
Copy link
Contributor

Update mocha to the latest version in Truffle. Lerna update wizard sure creates a lot of noise in the diff...

@eggplantzzz eggplantzzz changed the title Update mocha Dependency update: Update mocha Jun 19, 2019
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.005%) to 70.138% when pulling a3b5b66 on update-mocha into c5d5b7d on develop.

@cgewecke
Copy link
Contributor

NB: Mocha 6 seems to have undocumented breaking changes for reporters. For example it breaks eth-gas-reporter on a line similar to the one you've addressed with a3b5b66.

This might be something worth noting in the release notes since Truffle users could be consuming mocha in lots of different ways, via IDEs, various reporters etc.

@eggplantzzz
Copy link
Contributor Author

Oh I see you mean that deleted line might be something that users are currently consuming. @cgewecke, do you happen to know if this same module (ms) is still available but just at a different spot in the codebase?

@cgewecke
Copy link
Contributor

@eggplantzzz Mmm, I mean more vaguely that Mocha 6 seems to be breaking stuff and don't understand why.

For example on my side the full error looks like this and suggests that anyone extending the Base reporter will have a problem...

/Users/cgewecke/code/eth-gas-reporter/mock/node_modules/mocha/lib/reporters/base.js:319
  console.log(fmt, stats.passes || 0, milliseconds(stats.duration));
                         ^
TypeError: Cannot read property 'passes' of undefined
    at Gas.Base.epilogue (/Users/cgewecke/code/eth-gas-reporter/mock/node_modules/mocha/lib/reporters/base.js:319:26)
    at Runner.Gas.runner.on (/Users/cgewecke/code/eth-gas-reporter/mock/node_modules/eth-gas-reporter/index.js:125:10)
    at Runner.emit (events.js:194:15)
    at /Users/cgewecke/code/eth-gas-reporter/mock/node_modules/truffle/node_modules/mocha/lib/runner.js:829:12
    at /Users/cgewecke/code/eth-gas-reporter/mock/node_modules/truffle/node_modules/mocha/lib/runner.js:677:9
    at next (/Users/cgewecke/code/eth-gas-reporter/mock/node_modules/truffle/node_modules/mocha/lib/runner.js:290:14)
    at Immediate.<anonymous> (/Users/cgewecke/code/eth-gas-reporter/mock/node_modules/truffle/node_modules/mocha/lib/runner.js:334:5)
    at runCallback (timers.js:705:18)
    at tryOnImmediate (timers.js:676:5)
    at processImmediate (timers.js:658:5)

My reporter is a rewrite of Mocha's Spec Reporter and looks like this. It only imports the Base reporter and implementation seems consistent with the Mocha docs on third party reporters.

Issue thread about this at another mocha based project (mochawesome).

@cgewecke
Copy link
Contributor

And this is a proposed fix upstream from Mocha, even though the mocha PR which introduced this problem was a minor semver change.

@eggplantzzz
Copy link
Contributor Author

Hmm well maybe I'll go ahead and close this and hold off on upgrading up to 6 until things settle out a little bit more. Thanks for the info @cgewecke!

@eggplantzzz eggplantzzz deleted the update-mocha branch June 25, 2019 17:29
@cgewecke
Copy link
Contributor

Ok cool, thanks @eggplantzzz!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants