Support for ES2015 classes as tests containers in the exports UI

[ronjouch]

Hi.

I'm discovering and using Mocha for a fresh ES2015/2017 node app, and am enjoying the exports UI. To my eyes, it makes for concise and "normal-looking" test code, especially when used with ES2015 exports (which I get through Babel):

export function test_a() {/* ...*/}
export function test_b() {/* ...*/}

It works great and running Mocha tells me:

✓ test_a
✓ test_b

Now, say I want to group test_a and test_b into a SuiteOne test suite. With the current exports interface, if I understand correctly, I have to:

let SuiteOne = {
  test_a() {/* ... */}
  test_b() {/* ... */}
}

export {SuiteOne}

Yay:

SuiteOne
  ✓ test_a
  ✓ test_b

Awesome, but I was thinking:

• Now that ES has a class keyword...
• And following the established practice of test runners from OO-inspired languages (e.g. PyTest and many others)...
• And taking hints from recent versions of React which added support for using regular class instead of custom React.createClass() to declare components, enabling relying on a close-enough standard construct/syntax rather than custom APIs....

... I was wondering if it would make sense to make Mocha understand classes as tests containers:

export class SuiteOne {
  before() {/* ...*/}
  test_a() {/* ...*/}
  test_b() {/* ...*/}
}

export class SuiteTwo {
  test_c() {/* ...*/}
}

would output:

SuiteOne
  ✓ test_a
  ✓ test_b
SuiteTwo
  ✓ test_c

Wouldn't that look good / has this already been considered? Thanks for Mocha!

[TrejGun]

it would be awesome to have this :)

[ScottFreeCode]

Is there any particular advantage to doing this beyond looking more like other systems (mostly unrelated ones with different needs, goals and/or environments, I might add)?

Potential problems I can foresee include:

• Making sure we correctly detect when a function is actually a class (because class effectively returns the class's constructor, which is a function) while avoiding false positives, in all different JS environments that support class including a variety of transpilers and/or transpiler settings.
• Potential for someone to auto-convert their existing tests to the class-based exports style and have a test that is named constructor, which will become the class's constructor instead of being run as a test; can of worms hits the fan when this eventually happens, since the constructor that was meant to be a test may or may not blow up in a way that makes the problem obvious.
• The class constructor creates a new opportunity to run code at times that probably won't make sense to somebody (it's always going to be after the test file top-level code runs -- which itself may not be obvious to everyone -- but furthermore may or may not be after other test files' top-level code runs, and different people could have different rationales for expecting the one or the other that are most likely to show up in the form of wild XY problems) and might not even be consistent between Node and browser bundling. Note that the potential confusion here makes the value of being able to leverage this timing difference dubious, especially since the existing mechanisms for code ordering in test definition and test runtime appear to cover all possible needs already anyway. (All this assumes there isn't some portable way I've missed or forgotten to get the class's instances' prototype's functions without instantiating the class, anyway; if there is such an alternative, then if we took it the problem instead would be "why do my constructors not get run?")
• The identity of this is inevitably inconsistent/counterintuitive: either it's consistent with class instances and inconsistent with how this is used in all other Mocha test interfaces, or it's consistent with all other Mocha test interfaces and inconsistent with class instances. Note that this might also negate the possibility of leveraging class member variables in the tests -- not that we want to encourage such state carried on between tests anyway!
• Given that test functions would be moved to the prototype chain by this approach, should we look up the chain to base classes? If so, is there anything we need to filter out?
• Increased probability of somebody thinking Mocha's not doing something right when they discover their class-based tests don't work in environments that don't support class to begin with.

[ronjouch]

Is there any particular advantage to doing this beyond looking more like other systems?

@ScottFreeCode none for me, but I'd put it in a more positive way. To rephrase succinctly my initial post, I think that would improve the UX of the exports UI (and maybe help adoption) by supporting a popular pattern, and would do so following a path that is being used successfully elsewhere in the JS world: using the new class keyword to encapsulate a domain-specific container (the component in React, the test suite in this proposal).

(mostly unrelated ones with different needs, goals and/or environments, I might add)?

I disagree. Other test runners live under different environments, yes, but they share their core goal of running tests. In this context, UI familiarity is a feature, as it helps newcomers already familiar with the pattern from TestRunnerX when they move to Mocha for a new project.

---

Apart from this, I have nothing to add to your technical points, they all sound legitimate.

[boneskull]

I agree w/ most of @ScottFreeCode's points.

Most concerning is the behavior of this. In any Mocha Test or Hook, this is a Context object. I can think of two strategies here:

• Because of the restrictive nature of ES6 classes, we can't simply "merge" a class instance into an existing Context object; we'd have to instantiate the class first, then apply the Context constructor, and finally merge Context.prototype into to the instantiated object.

  But it would be problematic to even detect whether a file exports classes, since they are ultimately functions--so are before, beforeEach, etc. Node.js would have to simply add the class(es) to exports, since it can't use import/export--which isn't any different than what's happening now. Considerations for use with Babel could be tricky, because after transpilation, it may or may not look any different (often, named exports live in a defaults property, but not always?).

• Expose a Suite class, which must be an ES6 class, which would then provide the behavior of Context. You could then do class MySuite extends Suite, which would reduce the amount of fiddling as above.

  Mocha would also need to leverage Babel (which it doesn't) if we hoped to offer wide support for it, complicating its build and own test suites.

Either way, this would best be done in a new interface entirely; in the former, to avoid ambiguity, and in the latter, to expose the class. Looking at similar tools in other languages, you're going to want "TDD-style" setup and teardown functions instead of before, after, etc.

Increased probability of somebody thinking Mocha's not doing something right when they discover their class-based tests don't work in environments that don't support class to begin with.

This is a bigger issue than you'd think. Anything we can do to streamline and simplify reduces the support burden.

I think that would improve the UX of the exports UI (and maybe help adoption) by supporting a popular pattern
<snip>
UI familiarity is a feature

I'm not convinced we need to increase adoption of any interface other than the default bdd one--Mocha is based on RSpec/JBehave, after all. To that end, users familiar with these tools in Ruby and Java will have an easy time moving to Mocha. Providing a "class"-based interface, like JUnit, NUnit, etc., was never Mocha's goal. Interfaces other than bdd are provided mainly for convenience, but they also used significantly less, and receive fewer developer resources. One can argue (and I sometimes do) that other interfaces shouldn't be part of Mocha's core at all.

TL;DR: The current exports interface can't really bend in this way. Due to the maintenance/support requirements, I'd recommend this feature be exposed in its own module as an external interface (in other words, roll your own and npm publish it).

I'm going to close this, but it's a neat idea nonetheless.

[ScottFreeCode]

Tangentially (sorry for the late followup), I don't think we have documentation of how to write a custom interface; should we put together an issue to add that (or add it to a general needed-documentation issue such as #2282)?