Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for Jest in stylelint-rule-tester #815

Closed
kangax opened this issue Feb 25, 2016 · 10 comments
Closed

Add support for Jest in stylelint-rule-tester #815

kangax opened this issue Feb 25, 2016 · 10 comments

Comments

@kangax
Copy link
Contributor

@kangax kangax commented Feb 25, 2016

(opening issue here since stylelint-rule-tester doesn't have them enabled)

It would be great to add support for Jest in stylelint-rule-tester.

I think you guys might have guessed but we at Facebook are in the process of moving our old crappy linter to use wonderful Stylelint and PostCSS instead :) We're using Jest here for testing. What we've done for now is create a separate jest-stylelint-rule-tester which is essentially a stylelint-rule-tester but with API replaced to work with Jest (expect, it, etc.)

But of course, ideally, we'd love to add support for this "natively" so that other people can use it too and to have 1 source of truth instead of maintaining our own version.

So I wanted to throw this out there and get your thoughts. Would you be open to replacing Tape with Jest entirely? Or would we add support for Jest on top (and if so, is there anything Tape can do that Jest can't)?

@cpojer tells me that Jest 0.9.0 is like super fast so it might be even faster than what you currently have ;)

/cc @ngavalas

@davidtheclark
Copy link
Contributor

@davidtheclark davidtheclark commented Feb 25, 2016

I am definitely interested in figuring out a better system for rule testing. I will probably have some time later today to think about this a little more and maybe write down some of my thoughts about alternatives we might consider.

Thanks for raising the issue. And I'm really glad to hear that you're adopting stylelint at Facebook! That's awesome.

@davidtheclark
Copy link
Contributor

@davidtheclark davidtheclark commented Feb 26, 2016

Here's what I'd really like --- I'd like to have an abstraction of some kind (for running a CSS string against a stylelint rule and checking the warnings) that it itself test-engine-agnostic. Ideally, you could use that abstraction with whatever test engine you preferred -- Jest, tape, Mocha, Jasmine, AVA, etc. -- either directly or with a very light wrapper that wired up the particulars of the test engine.

So instead of just switching from tape to Jest, or AVA, or something else, I'd really like to build the abstraction that would make testing rules easy for everyone. If that's doable. I'm trying to brainstorm possible ways of approaching this problem. Input, feedback, help is all very appreciated.

We could use Node's assert() to make assertions and hope the testing framework can handle that kind of assertion. But tape can't, I think, and maybe others can't, either. Also, I've had async-related headaches trying this (below).

We could make a test-describing schema that is just arrays of test-case descriptions, which should or should not trigger warnings. But then we still need an abstraction that interprets the schema and applies some assertions to it. And so you change the syntax of the tests without solving the fundamental problems.

One big problem standing in the way is asynchronicity. We run the string through PostCSS (using stylelint as a plugin) then look at the generated result for warnings. That process is async. The trouble is that it seems each testing framework deals with async in a different way, so I'm having a hard time deciding how to approach that and come out with a syntax that's succinct enough to be valuable. (Being able to write tests as succinctly as we can currently is essential. That's the only way I've stayed sane writing so many tests.)

Another problem is getting good reports when something fails --- helping you to understand the failure and track down the specific test that failed. Creating an abstraction over any test framework seems to cause a problem here: they're all calibrated to rely on the location of their assertions, or something like that.

The last problem is that there are over 10000 unit tests already in this repo, and if we were to change the way the tests themselves are written, we'd really really really want to be able to update the tests automatically, with some regexp find-replace or something, rather than manually ...

Again, any ideas welcome. Before considering switching from tape to another test engine I'd really like to see if it's feasible to create something like the abstraction I'm getting at.

@cpojer
Copy link

@cpojer cpojer commented Feb 26, 2016

We should also open source our jest adapter, for a start @kangax.

@davidtheclark you might find this talk valuable: https://www.youtube.com/watch?v=d0pOgY8__JM :)

@davidtheclark
Copy link
Contributor

@davidtheclark davidtheclark commented Feb 26, 2016

Thanks for sharing the talk @cpojer. Great job! You're right that I shouldn't have to be live in fear of a change to the test syntax when a codemod could help :)

@davidtheclark
Copy link
Contributor

@davidtheclark davidtheclark commented Mar 18, 2016

@kangax , @cpojer : With the merging of #824 I think it should be really straightforward to write a rule tester for any engine. Please me know how that goes with Jest when you have a chance to try it out.

@jeddy3
Copy link
Member

@jeddy3 jeddy3 commented Apr 9, 2016

Issue triage: closing this.

Please let us know how it goes by posting to this closed issue. If you run into any problems though, please open a new issue.

@jeddy3 jeddy3 closed this Apr 9, 2016
@dangreenisrael
Copy link

@dangreenisrael dangreenisrael commented Jul 5, 2019

Any progress on this?

@laras126
Copy link

@laras126 laras126 commented Jul 22, 2019

@dangreenisrael It looks like there is some related conversation here: #3982

@laras126
Copy link

@laras126 laras126 commented Jul 23, 2019

Perhaps helpful for those arriving on this thread:

At the time of my research, Stylelint itself is not using stylelint.createRuleTester for its internal Jest tests - the documentation is a bit misleading and out of date. It has a different testRule implementation here: https://github.com/stylelint/stylelint/blob/master/jest-setup.js

A similar approach (FWIW, I copy-pasted this into my project and it worked!): https://github.com/kristerkari/stylelint-high-performance-animation/blob/master/jest-setup.js

This is another approach using the Stylelint methods instead of the testRule approach in Jest that is a bit more manual: https://github.com/ierhyna/stylelint-no-indistinguishable-colors/blob/master/__tests__/index.js

@ntwb
Copy link
Member

@ntwb ntwb commented Jul 24, 2019

I'd be more than happy to see this updated, I started but have had little time recently to progress this further is to create a shared Jest configuration for stylelint at https://github.com/stylelint/jest-preset-stylelint

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants