Rules_nodejs: Add mocha_test rule

[chrisleck]

Similar to jasmine_node_test, create a mocha wrapper for running simple TS / javascript / node unit tests.

Other similar nodejs rulesets for bazel already support this: https://github.com/pubref/rules_node#mocha_test

[pscarey]

Any issues using something like:

def mocha_ts_test(name, data = [], **kwargs):
    """Runs mocha on TS files using ts-node."""
    data += [
        "@npm//ts-node",
        "@npm//mocha",
        "@npm//sinon",
        "@npm//chai",
        "@npm//@types/node",
        "//:tsconfig.json"
    ]

    args = [
        "--reporter list",
        "--require ts-node/register/transpile-only",
        "**/*.test.ts"
    ]

    nodejs_test(
        name = name,
        data = data,
        entry_point = "npm/node_modules/mocha/bin/mocha",
        templated_args = args,
        expected_exit_code = 0,
        **kwargs
    )

We've implemented that in a defs.bzl alongside the WORKSPACE file, with:

load("//:defs.bzl", "mocha_ts_test")

mocha_ts_test(
    name = "assert",
    data = glob([ "src/assert.*", "src/has.ts" ])
)

It just works by running any .test.ts file which is included in data. The typical usage would be 1 test file, and any required source files + other dependencies.

Let me know if this is the right way to go, and I'll look at tidying up the above & a pull request etc.

I don't know if you want to end up with a test rule for every test runner and/or bundling program. Surely anything provided through NPM with a CLI could fit into the above format?

Another alternative would be a short example of how to do something like the above to help people new to Bazel.

(As a side note r.e. other test runners - I had a lot of headaches trying to make Jest work, including postinstall patches to fix it's symlink handling behaviour as it would just hang with no errors due to the Bazel symlinks. Was kinda buggy and slow, not really sure why.).

[chrisleck]

My quick stream-of-conciousness thoughts:

• Thanks for digging into this!
• It should support yarn in addition to npm - don't assume or hard-code one or the other.
• Test sources should appear in the src field, not the data field. data should be reserved for actual data files needed by the tests.
• A lot of mocha users send in a mocha.opts file for rules & bootstrapping. Could we have a special field for that to make it easy to use?

[pscarey]

Cool, I'll do some more work on it if there's nothing fundamentally wrong with the approach.

1. Easy - the 'npm' is just a reference to the workspace used by yarn_install and npm_install
2. OK
3. I'll look into configuration options and the best ways to offer them.

[alexeagle]

After doing some cleanup work on #383 I found that mocha_node_test was indistinguishable from jasmine_node_test from a user point of view.

Putting it another way, if we were to rename jasmine_node_test to node_unit_test, then we could swap out Jasmine for Mocha behind the scenes as a non-breaking change.

So that leads me to ask, why do you want a mocha test rule? Is there some test you've authored that can't execute under jasmine_node_test? Would renaming jasmine_node_test to node_unit_test be the simplest fix for this issue?

[pscarey]

Can all tests which run in Jasmine run in Mocha? No, as Jasmine is 'batteries included' and Mocha requires you to include Chai, Sinon, etc for expectations and mocking etc.

Can all tests which run in Mocha run in Jasmine? Possibly, simple tests do. I don't think that APIs are the same despite being similar. Likely edge cases which break.

• Surely therefore it'd be a breaking change to swap Jasmine for Mocha behind the scenes?
• What about other test runners?

Our goal is to run Typescript tests (.test.ts) files on Typescript source files (.ts), by calling a test runner with ts-node. We could compile tests + sources to JS and run the tests as JS too, which would add 1-2 extra rules per unit test.

Is it preferable to have more general support, i.e. node_unit_test which takes a test_runner argument, (one of mocha, jasmine, etc with a default), and a list of args, or more specific support, e.g. mocha_node_test which supports mocha_opts directly.

For example, a generic test runner:

node_unit_test(
    name = "dot_spec_test",
    srcs = ["foo.spec.js"],
    node_modules = "//internal/test:node_modules",
    test_runner = "mocha",
    args = [
        "--ui",
        "bdd",
    ],
)

N.B. if you wanted to pass in files to the test runner, e.g. mocha.opts, mock files as other dependencies, you'd need to use $(location :mocha.opts) so that the argument gets expanded, which is pretty opaque for users.

Or a mocha-specific:

mocha_node_test(
    name = "underscore_spec_test_with_mocha_opts",
    srcs = ["foo_spec.js"],
    mocha_opts = ":mocha.opts",
    node_modules = "//internal/test:node_modules",
)

Since it's easy enough to write an additional wrapper around the nodejs_test function - you could just choose to offer a set of examples of how to use various test runners with that, and encourage people to DIY.

[alexeagle]

Sorry, I'm still missing the answer to a basic question: if all known tests run under Jasmine, then what is the motivation to use mocha instead?

[chrisleck]

Not all known tests run under Jasmine. My team has invested heavily in mocha and would like to use a mocha-native test command that includes mocha_opts for bootstrapping.

[alexeagle]

@chrisleck of course you're free to use mocha, and your team can maintain a rule that does that.

For considering upstreaming it to this repo, however, there's a high bar because our team will have to maintain and support the rule from now on. That's why I'm trying to carefully understand the tradeoff.

Could you give an example of a test that runs under Mocha but not under Jasmine? How typical do you think such an example is?
Also could you give more motivation for why to expose Mocha's options? Is it only for presentation of the results?
Thanks!

[haikalpribadi]

Please consider the discussion in issue #394 when naming this new test rule.

[chrisleck]

I can't include my team's tests directly here, but we utilize Mocha's ability to create your own custom runner interface (using the confusingly named --ui parameter) for performing async tests with our fake timer implementation, async/await, and native promises.

Also, our mocha.opts includes bootstrapping code that all of our 10000+ unit tests require to run:

--max-old-space-size=4096
--allow-natives-syntax
--expose-gc
--check-leaks
--full-trace
--trace-warnings
--exit
--recursive
--reporter dot
--slow 75
--timeout 3000
--ui bddTemporal

--require ts-node/register
--require modules/dev/test-harness/bootstrap.ts
--require modules/browser/react/bootstrap.js

[Globegitter]

@alexeagle Yeah it is a tricky one indeed, but here is a very basic comparison of mocha vs jasmine: https://raygun.com/blog/mocha-vs-jasmine-chai-sinon-cucumber/ so feature-wise they are very similar, but syntactically they are different, so while it might be possible to migrate things to jasmine I think it is going to be a big task for big test suites. So I think the question comes down to, should we support the 3 major test frameworks, jasmine, mocha, jest here in rules_nodejs and increase the burden of the core-team and the community, or increase the burden of the people who use anything else than jasmine and want to migrate to Bazel? Given that migrating to bazel is not always an easy task for big code bases I would think it is worth doing everything we can to make it as easy as possible for people to migrate and not ask them to also learn another test framework.

Would it be possible maybe to have kind of a middle ground @alexeagle? rules_docker e.g. has a contrib directory: https://github.com/bazelbuild/rules_docker/tree/master/contrib for user contributed rules. Maybe we could do something similar? Rules there are contributed by the community and are kept in the main rules as there are enough people making use of it and it will help with visibility and most likely also help with bazel adoption, but we can clearly say that those are community contributed rules so they do not have the same level of support by the core team and are expected to be mostly supported by the community. What do you think?

[alexeagle]

I finally got around to trying this. A mocha_test rule seems like overkill for us to ship with core rules_nodejs when you can implement your own thing so easily. Here's a working mocha_test

load("@build_bazel_rules_nodejs//:index.bzl", "nodejs_test")

def mocha_test(name, args=None, srcs=[], deps=[], **kwargs):
    if args == None:
        args = ["$(locations %s)" % l for l in srcs]

    nodejs_test(
        name = name,
        entry_point = "@npm//:node_modules/mocha/bin/mocha",
        data = srcs + deps + [
            "@npm//mocha",
        ],
        templated_args = args,
    )

[alexeagle]

adding a mocha example in https://github.com/bazelbuild/rules_nodejs/pull/1216 to close this

[alexeagle]

A mocha_test is now in the examples

[chrisleck]

@alexeagle - in your example, how does one refer to a mocha.opts config file located in a subdirectory off the root from a centralized macro? In other words, I need to execute mocha --opts ${repo_root}/test/mocha.opts ${srcs}. It seems like I can override the auto-generated mocha_test rule in the same way the jest_test example here does, but I'm not clear on how to have it point to a global config file.