Upgrade mocha

[koddsson]

Update Mocha from 8.2.1 to 10.2.0!

[changeset-bot]

🦋 Changeset detected

Latest commit: 96758fc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 7 packages

Name	Type
@web/dev-server-rollup	Minor
@web/test-runner-commands	Minor
@web/test-runner-junit-reporter	Minor
@web/test-runner-mocha	Minor
@web/test-runner-visual-regression	Minor
@web/dev-server	Patch
@web/test-runner	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[Westbrook]

The breaking changes in v9 are where Dependabot failed us on this too, but since then it seems that Mocha has moved to v10 (npm listing). Would be great to move all the way through to that version, here.

[koddsson]

The breaking changes in v9 are where Dependabot failed us on this too, but since then it seems that Mocha has moved to v10 (npm listing). Would be great to move all the way through to that version, here.

For sure, I did that initially, but just had so much breakage that I thought I'd try to do this in smaller steps. Trying to figure out what breaks still 😄

[Westbrook]

Does this count as a breaking change? Likely.

Should we target #2180 for this PR?

[Westbrook]

Looks like you may just be missing these two https://github.com/koddsson/web/blob/update-mocha/package.json#L53-L68 and then the breaks of the change will open unto you?

[koddsson]

Looks like you may just be missing these two https://github.com/koddsson/web/blob/update-mocha/package.json#L53-L68 and then the breaks of the change will open unto you?

Yup! I've done this locally but didn't push it since it was broken. But I can push so we can all see the error in CI :)

[43081j]

had a quick look, there's still something wrong but the first problem is this:

web/packages/test-runner-mocha/src/mochaSetup.ts

Lines 4 to 5 in 8999617

	const BaseReporter = (mocha as any).Mocha.reporters.Base;
	class SilentReporter extends BaseReporter {}

mocha 10's window object is a Mocha instance. the other exports no longer live on there, so probably unlikely we can get at Base anymore via globals (rather via esm).

instead you can just do:

class SilentReporter {
  done() {
    return;
  }
}

which is the interface of a reporter near enough so should solve that problem

[43081j]

i think you need to still use the global since its testing that the setup worked correctly (i.e. its testing the window object has mocha on it)

but even then i think ci will fail 😭

[43081j]

@koddsson got it:

const mocha = (window as any).mocha;
const mochaExports = (window as any).Mocha;

class SilentReporter extends mochaExports.reporters.Base {
}

the exports got moved to window.Mocha while the mocha instance is at window.mocha

if you're curious - it didn't work before because of a couple of things:

• done has the signature done(failures, fn) and you need to call fn(failures) ultimately to finish the test run. we weren't doing this in my suggestion so it just timed out (or you just don't implement done and it calls fn(failures) for you)
• Base is actually what populates test.err so you can't get away with a bare class, it has to inherit Base still (or you write your own error handling)

[koddsson]

@43081j Let's hope this works! 🤞🏻

[koddsson]

Amazing work here @43081j! I had done some changes with the BaseReporter locally, but never got it to work. Really happy you were able to figure this out <3 <3

[43081j]

Awesome, glad it worked!

So the original code wasn't far off actually

Pretty much window.mocha.Mocha moved to window.Mocha

[koddsson]

Looks like we are all green! Let me know what you think, and feel free to merge at your own convenience.

[Westbrook]

Exciting! Need to do a double check on a PC in the morning, but looking good. Nice merges we’ll get one last canary, and assuming there’s no issues, and no last minute feedback to the contrary, we’ll go to master with the whole set of change! 🎊

[Westbrook]

LGTM!

This merge should auto release a new canary, and with a little testing there, maybe we're good to go?

Ecosystem wise, are most other people also leveraging this through @open-wc/testing-helpers? Or is it just me? We'll need to update those as well.