Feature Request: Support for intended fail-scenario

[shani-terminus]

I would like a way to verify a fail scenario to insure code properly messages invalid parameters.

For example:

@mixin theme-color($property: 'background-color') {
  // Verify an allowed property was passed in
  @if not(($property == background-color)) and not(($property == color)) {
    @error 'The `theme-color` mixin only accepts `background-color` or `color`';
  }
---- do stuff ----
}

Test:

@include test('should fail with invalid property') {
    @include assert-fail {
      @include output {
        @include theme-color(foo);
      }
      @include expect {
        @include fail-message('The `theme-color` mixin only accepts `background-color` or 'color`');
      }
    }
  } 

Today when I attempt a test like this, I get the fail-message in the terminal, however, the test itself fails and my other tests don't run.

[mirisuzanne]

There is no way to build this into True's testing-syntax directly, since the @error declaration stops Sass compilation. It would require changes to how you write functions and mixins in the first place. You can see how we've done that for both True and Susy – by passing all our errors through a function or mixin that toggles between returning the error message as a string (in tests), or triggering the Sass @error (in producion).

The best we could do is provide those functions/mixins as a plugin that you can include in your primary Sass, rather than copy-pasting the functions yourself. That should be doable. In the meantime, feel free to copy/paste, and let me know if you have questions.

[shani-terminus]

Thanks! I will look into implementing in our next sprint.

[DesignByOnyx]

Would you consider allowing special comments to wrap around tests which should throw? Tests might be written like this:

@include describe('some-mixin') {
  @include it('should work') {
    /* normal test omitted for brevity */
  }

  /** START_EXPECT_ERROR: Maybe the expected error message could go here */
  @include some-mixin(bad, params);
  /** END_EXPECT_ERROR */
}

You would then have a multi-phase parse:

1. Strip out all EXPECT_ERROR blocks - easily done with regex.
2. Replace each error block with some /* unique_token */ so it can be re-injected later
3. Parse the file without any error blocks
4. For each error block... one at a time... inject the code and parse again using try/catch
5. Validate the error message inside the catch block

I would gladly help with this if this seems like a viable solution. My main objective is to allow people to write SASS and not have to learn a separate error mixin and global variables. Thanks again for such a wonderful tool.

[mirisuzanne]

@DesignByOnyx I'm open to discussing something like that.

While it requires less change to your project code (no special mixin), it also removes the ability to run your tests in pure Sass. I wonder if we could work around that by doing something like (pseudo-sass):

// - regex should work just as well with a mixin, so we don't add a completely new syntax
// - by default, it won't compile. Users or True can pass in $compile: true when it should
// - if a message is given, error message must match
// - if no message is given, any error will pass the test
@mixin assert-error(
  $message: null,
  $compile: false
) {
  /* we can supply the necessary output comments here… */
  @if $compile {
    @content;
  } @else {
    /* whatever output we want for pure-Sass compilation  */
  }
}

// usage…
@include assert-error(`<optional error message>`) {
  /* code-block that should error */
}

// also handle warnings?
@include assert-warn(`<optional warning message>`) {
  /* code-block that should warn */
}

[DesignByOnyx]

Would True loop through each assertion and dynamically set $compile: true one at a time? If so, I think that's an excellent idea. Personally, I wouldn't even document the second parameter and leave it to True. There's talk of introducing @try/@catch directives into the language itself - though it seems a long ways off, and I'm not quite sure if the language needs more API just for the sake of testing.

[mirisuzanne]

Would True loop through each assertion and dynamically set $compile: true one at a time?

Yep, exactly.

• The second parameter might be useful when authoring tests?
• If not, we should probably remove it as a parameter entirely, and use a global variable instead.

There's talk of introducing @try/@catch directives into the language itself

Yeah, Nex3 asked me if it would be useful, and I pointed her to our current ugly workaround as the clear use-case. Not sure what will come of it, but agree we shouldn't count on it any time soon.

Next Steps: Do you want to work on a PR for the entire thing? Or helpful for me to start some of the Sass end? Up to you.

[DesignByOnyx]

If you can give some pointers on where to start, I'd appreciate it as it would save me some time. The biggest unknown to me is how to figure out the describe/it block in which the assertion was made and appending the results to the correct context. Let's start with a simple block of code for discussion:

$not-a-color: #gggggg;

@include describe('some-mixin') {
  @include it('should work') {
    /* normal test omitted for brevity */
  }
  
  @include it('should fail on bad params') {
    @include assert-error('Must pass a valid color value') {
      @include some-mixin($not-a-color);
    }
  }
}

1. On first run, the assert-error mixin is a no-op and it never renders its $content, thus never triggering the error.
2. Then, we must somehow discover each assert-error call and render the $content inside of a try/catch:

   const errorAssertions = findAllErrorAssertions(sourceFile);
   errorAssersions.forEach(assertion => {
     const data = sourceFile.replace( /* do the thing to render the mixin $content */);
     try {
       sass.renderSync({ data });
       // figure out the describe/it context and `assert(false)` as the error was not triggered
     } catch(ex) {
       // compare error messages if one was provided
       // figure out the describe/it context and `assert(true)` if all looks good
     }
   });

[mirisuzanne]

I can stub out the Sass in a branch so you have a real sample to work from. I think the JS currently works by parsing that CSS output, so:

1. Yes, but also the assert-error mixin will generate a comment in the output CSS. We can play with what information needs to be in that comment, but I'll start with something basic: the module, test, assertion-type, and assertion-message.
2. Sounds right. I'm not an expert on the JS side of this project - so we can pull in @jgerigmeyer for questions or review on that side of things.

[DesignByOnyx]

I've been digging through the code I think I figured out a good strategy for this and am going to take a stab at it. Feel free to save your time on stubbing things for right now as I think what I'm doing should work well and not require any difficult parsing.