Make RuleTester test runner agnostic

[jfmengels]

Hey guys!

I have a few ESLint plugin repos I'm maintaining, where we use AVA as the test runner instead of mocha (including eslint-plugin-ava 😄). What we did until now was to wrap the whole RuleTester call in one AVA test, which made the test feedback pretty bad (it's all good or all fail, you don't know which one crashed, and the error messages were simplistic), just like if you would simply run the test file using node instead of mocha.

I noticed that by overriding the RuleTester's before and itmethods, I could create proper test suite à la mocha, and created https://github.com/jfmengels/eslint-ava-rule-tester. It feels a bit hacky and fragile, so I would have liked to know if there was a better way, or if we could introduce one, to make RuleTester more test runner agnostic.

I provided screenshots of actual behavior with AVA and the behavior with eslint-ava-rule-tester in this PR (source code available there if needed) avajs/eslint-plugin-ava#105.

I'm not sure about the solution to offer, but it would probably be something like adding a new parameter to RuleTester, which could be either an object with keys describe and it (which would be functions that create tests), or a new function that returns that.

var test = require('ava');

var avaTestCreator = {
  describe: function(test, method) {
    return method.apply(this);
  },
  it: function (text, method) {
    test(text, method);
  };
};

var ruleTester = new RuleTester({
    env: {
        es6: true
    } // and other options
}, avaTestCreator);

(cc @sindresorhus @twada @jamestalmage)

Oh, and by the way: Love the work that you guys are doing, thanks so much for the work you're putting into this ❤️, I don't express it often enough.

[ilyavolodin]

RuleTester used to have a hard dependency on Mocha, but it was removed to satisfy some plugin requirement of using Jasmine instead. It works by the fact that both Mocha and Jasmine use the same syntax (describe, it). I'm not really sure how we could make it completely framework agnostic. Even if we were to expose two variables that you could override, it would work for every use case (for example, QUnit, doesn't really have anything like describe, just it).

I think what you are currently doing is not too bad. It's a bit hacky, but then what you are trying to do is a bit hacking in on itself:-) We could add a second parameter to RuleTester constructor, but it wouldn't really be all that much different than what you are already doing, and it would also be harder to setup defaults for it (I think).

P.S. In general I'm pretty interested in this, due to the fact that our unit tests are taking a while to run (since we have so many of them). If we could have an option of painlessly switching to AVA at some point, and running them in parallel, it might significantly improve our build time.

[jfmengels]

for example, QUnit, doesn't really have anything like describe, just it

QUnit has modules, which seem similar (could be wrong). AVA doesn't have a describe. What I did in eslint-ava-rule-tester was this: store the description of the describe and prefix it to every "it". It could have been an even simpler function that just calls the passed method and that would work fine.

it wouldn't really be all that much different than what you are already doing

Yeah, probably not. And I would probably still need to install a package that does the bridge between RuleTester and AVA, so that I wouldn't have to write this in every test file. But at least I would have some greater garantees that this wouldn't break in some ESLint internal refactor. Right now I'm just hoping that you don't move this part too much, but with some sort of hook, I'd feel safer because I know I could at least rely on semver.

In general I'm pretty interested in this

Yeah, saw your comment in the roadmap issue :) If you're curious and want to have a look, I know at least 3 projects using RuleTester and AVA: eslint-plugin-ava, eslint-plugin-lodash-fp, eslint-plugin-xo.

[ilyavolodin]

I would be fine with adding another optional parameter to the RuleTester constructor that could supply overrides for describe and it. It would still not make RuleTester portable, since you are locked into Chai, but that would at least provide an easier way to use it with a different test runner. I think if accepted, this would be a pretty low priority for the team, so you might need to submit PR yourself, or it might take a while for somebody on the team to get around to it.

[jamestalmage]

It would still not make RuleTester portable, since you are locked into Chai

RuleTester appears to only depend on assert. I don't see any Chai .should usage in there either. Perhaps I'm missing something.

I would be fine with adding another optional parameter to the RuleTester constructor that could supply overrides for describe and it.

👍 - adapter or testRunnerAdapter would be good names for this option.

Assuming I am right about assert above, it might be worth also allowing a replacement assert API (it would be the adapters responsibility to be API compatible with core assert). With that, AVA could wrap the assert API so it participated in assertion planning and power-assert output beautification.

[jfmengels]

this would be a pretty low priority for the team

Yeah, understandable, especially since I do have a work around for this. If this gets accepted, I may try to submit a PR, depending on how complicated this will be.

[nzakas]

Sorry, trying to get caught up here.

First, RuleTester is already library-agnostic. If you don't have Mocha installed, it will still run the asserts. It also "just works" with Jasmine, since it defaults to using global describe and it if found, and just uses an empty function if not.

You can already override describe and it on a RuleTester to do whatever you want, so I'm not sure why a second argument to the constructor that overrides describe and it would be helpful.

I did just skim this over briefly, so do let me know if I'm missing something important.

[jfmengels]

@nzakas The main thing would be semver garantee.
Right now, I'm doing what you just said, but I'm hacking it, and I have no garantee that the ESLint team will not remove it, as you might consider it as something internal.

Then as @jamestalmage mentionned, if we could inject an assert object, we could potentially do some more cool stuff.

@ilyavolodin I just checked what @jamestalmage said, and I don't see any use of chai in RuleTester. It's used in your internal tests, but it's not the assertion API used by RuleTester.

[ilyavolodin]

Ah, you are correct. assert used to come from chai in the initial version of the RuleTester, but not anymore.

[jamestalmage]

Implementing this would greatly improve, this situation .

At a minimum, let's add a few tests validating what happens when you replace it and describe.

[nzakas]

@jfmengels RuleTester is part of our exported API, so it follows semver when it comes to changes.

Allowing assert to be overwritten seems like an easy thing to do if that's helpful.

@jamestalmage we happily accept pull requests with more tests.

[nzakas]

@jfmengels can you refine your proposal based on this discussion?

[jfmengels]

My initial proposal is basically solved if you tell me that you consider the it and describe functions of RuleTester as part of the public API. If you tell me I can safely override those functions until at least ESLint v3, then I'm fine with this. @sindresorhus @jamestalmage do you agree?

On the proposed assert object injection, @jamestalmage is better suited to make a proposal (but might be better to put it in a new issue).

Thanks for answering! (and sorry for not responding earlier btw)

[nzakas]

Yeah, it and describe are part of the API. We should probably document them specifically.

@jamestalmage if you want to propose something around assert, please open a new issue.

[pedrottimark]

@jfmengels Could you = we who submits a pull request to document it and describe 😀

[jfmengels]

@pedrottimark I can give it a shot. Would this be a good spot to put it http://eslint.org/docs/developer-guide/working-with-plugins#testing? There's no other section dealing with it afaik.

[nzakas]

@jfmengels yeah, that's a good spot. Probably just a section for customizing RuleTester there would do.