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

Sass Syntax - Tests progress #166

Closed
41 tasks done
bgriffith opened this issue Sep 13, 2015 · 7 comments
Closed
41 tasks done

Sass Syntax - Tests progress #166

bgriffith opened this issue Sep 13, 2015 · 7 comments
Labels

Comments

@bgriffith
Copy link
Member

bgriffith commented Sep 13, 2015

Having had a few Sass Syntax related issues popping up, I think it would be a good move to create tests for that syntax too.

I've started going through the current tests and have created quite a few .sass equivalents. So far there has been a few tests that have failed but the majority are ok.

By testing both the SCSS and Sass syntax I feel we will then be able to confidently say that we have cross syntax support.

I'll update the list below as I go.

The rules

Rules that are N/A

  • trailing-semicolon
  • space-before-brace
  • no-empty-rulesets
  • brace-style
  • one-declaration-per-line
@DanPurdy
Copy link
Member

Awesome, I started having a look at this after we spoke this morning, but looks like you've got further ahead than me. Do you wanna explain a little how you're going about it, I've got a few rules nearly done here that i'd like to get the sass tests up and running for too, be good to have them in the same format as yours.

@bgriffith
Copy link
Member Author

Sure, it's very simple and happy to alter if you've got a better suggestion.

All I'm doing is creating .sass versions of the .scss files (with any adjustments if required) and updating the mocha test file to test for both.

Ideally we should be writing the actual rules in a way that's syntax agnostic so in my mind we shouldn't have two separate tests but simply improve the current one's to take into account any issues (currently not too many).

Example below;

'use strict';

var lint = require('./_lint');

describe('no extends - scss', function () {
  var file = lint.file('no-extends.scss');

  it('scss - enforce', function (done) {
    lint.test(file, {
      'no-extends': 1
    }, function (data) {
      lint.assert.equal(1, data.warningCount);
      done();
    });
  });
});

describe('no extends - sass', function () {
  var file = lint.file('no-extends.sass');

  it('sass - enforce', function (done) {
    lint.test(file, {
      'no-extends': 1
    }, function (data) {
      lint.assert.equal(1, data.warningCount);
      done();
    });
  });
});

The only issue I've found with this method is that you have declare file within the assertion and so there's some duplication of that when you have lots of assertions. Thoughts?

@DanPurdy
Copy link
Member

This will be ok to start with but I think the duplication will become a little unmanageable for the rules where there are lots of test cases.

The tests should give the same results for both scss and sass, but I guess seen as some of the rules won't apply to the sass side of things it adds a small challenge there, for now something as simple as setting the file variable to an array of the 2 different syntaxes and then just looping the tests could work for now?

i.e. one loop for the whole test sweet that just runs each tests against each syntax

DanPurdy added a commit to DanPurdy/sass-lint that referenced this issue Sep 14, 2015
@DanPurdy
Copy link
Member

Followed your lead I believe for this new rule, we can revisit once everything is up and running again?

DanPurdy added a commit to DanPurdy/sass-lint that referenced this issue Sep 14, 2015
@DanPurdy
Copy link
Member

You should look to get this up as a PR soon preferably before you go any further, we can then keep track a bit better of what still needs to be done and any issues we find in separate issues. Also it'll give us a nice base from which to get sass tests as the norm for any further rules being written.

@bgriffith bgriffith changed the title Tests for Sass Syntax Sass Syntax - Tests progress Sep 17, 2015
@bgriffith bgriffith self-assigned this Sep 18, 2015
@bgriffith bgriffith added this to the 1.3.0 milestone Oct 7, 2015
@DanPurdy DanPurdy modified the milestones: 1.4.0, 1.3.0 Oct 8, 2015
@DanPurdy DanPurdy removed this from the 1.4.0 milestone Oct 28, 2015
@DanPurdy
Copy link
Member

Going to wait for the latest version of gonzales-pe to rewrite the indentation. Moving this out of 1.4.0 for now.

@bgriffith bgriffith removed their assignment Oct 28, 2015
donabrams pushed a commit to donabrams/sass-lint that referenced this issue Nov 19, 2015
donabrams pushed a commit to donabrams/sass-lint that referenced this issue Nov 19, 2015
@bgriffith
Copy link
Member Author

Indentation is now good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants