search

tomasbasham / ember-cli-scss-lint

An ember-cli addon to integrate sass-lint for standards adherence and improved style consistency
MIT License
7 stars 6 forks source link

Making ember test fail when linting errors exist #10

Closed simonc closed 7 years ago

simonc commented 7 years ago

Hi,

It may be a mistake on my part but I'm trying to configure ember test to fail when there a scss-lint errors and can't find a way to make it happen.

When I run ember test I get the following trace:

bootstrap-sassy config: some JS loaded [dropdown,modal], glyphicons disabled

[scss-lint] components/_stay-list.scss:1:1 - SpaceBeforeBrace: Opening curly brace `{` should be preceded by one space
Built project successfully. Stored in "…/customer-advising-dashboard/tmp/core_object-tests_dist-gLpwPNjD.tmp".

ok 1 PhantomJS 2.1 - ESLint - modules/ember-cli-has-many-through/macros/has-many-through-non-object.js: should pass ESLint
…
1..154
# tests 154
# pass  154
# skip  0
# fail  0

# ok

As you can see there is a linting error but the tests continue anyway and return a 0 status code.

Is there some sort of config option to enable errors?

Thanks! ☺️

tomasbasham commented 7 years ago

No it is not a mistake on your part. This addon does not support that functionality. I do need to make a few bug fixes and feature requests over this weekend. Do you feel this is a feature that would be useful to others? If so I'd be more than happy to try include it.

simonc commented 7 years ago

I think it could be if it's opt-in/out. Our use case is the following: we deploy on Heroku, using CodeShip to run the tests and linters.

We don't want to accept PR that don't respect the linting guidelines so having the linter make the CI fail would be awesome ☺️

I can't present a PR to you because JS is really not my strong suit but I'm available if you need me to test our CI pipeline and tell you how it goes :wink:

Thanks for your rapid answer!

simonc commented 7 years ago

@tomasbasham Hello :blush: Why did you close the issue?

tomasbasham commented 7 years ago

Because I have resolved it. I believe

simonc commented 7 years ago

Oh ok! Good to know 😄

simonc commented 7 years ago

Thank you!

tomasbasham commented 7 years ago

No problem. I probably should have mentioned it before closing the issue.

If you go back to the README I have made the decision to transition over to using sass-lint; a pure JS version of the Ruby scss-lint. I have done this because the Sass core team is now building Sass in Dart instead of Ruby, and will no longer be maintaining the Ruby implementation. Since scss-lint relies on the Ruby Sass implementation, this means it will eventually not support the latest Sass features and bug fixes.

This does require you to translate your .scss-lint.yml file into a .sass-lint.yml file. You can do that swiftly here.

Having done this I also managed to implement a test generator that when run using ember test will cause it to flag linting test failures.

simonc commented 7 years ago

That's pretty cool 😄 (And good to know about the change regarding Sass, thanks for the heads-up!)