Add specs for content-exists in control statements #1361
Conversation
It's not enough to check the head of the stack to determine if we're within a mixin. Spec sass/sass-spec#1361 Fixes #2842
It's not enough to check the head of the stack to determine if we're within a mixin. Spec sass/sass-spec#1361 Fixes #2842
It's not enough to check the head of the stack to determine if we're within a mixin. Spec sass/sass-spec#1361 Fixes #2842
It's not enough to check the head of the stack to determine if we're within a mixin. Spec sass/sass-spec#1361 Fixes #2842
It's not enough to check the head of the stack to determine if we're within a mixin. Spec sass/sass-spec#1361 Fixes #2842
There was a problem hiding this comment.
Hey Michael! I'm trying to (gradually) clean up and normalize the way specs are laid out in this repo. I've written some initial guidance on how to to organize specs in the new README, and I'm hoping to flesh that out into a full-fledged style guide soon, but in the meantime I guess I'll just comment ad hoc on pull requests.
-
Please merge all these specs into a single
content_exists.hrxfile. Generally speaking, specs for a given feature should be in as few HRX files as possible without going over ~500 lines per file. -
HRX files function as virtual directories, so there's no need to move
basic.hrxanderror.hrxintobasicanderrordirectories. -
Please split each assertion ("
content-exists()returnstruewhen called from within a mixin with@content" and "content-exists()returnsfalsewhen called from within a mixin without@content") into a separate spec. Now that we have HRX, it's feasible to write small tightly-scoped specs that test one thing each without creating a million files, which is generally a better testing practice.
| } | ||
| } | ||
|
|
||
| .should-be-true { |
There was a problem hiding this comment.
Nit: I'd call these classes .with-content and .without-content. It's already clear what the expected result is, since it's right there in the output file, so it's more useful to document what's meaningfully different about how they're invoked.
Actually, when you split these into separate specs, you should probably put with_content and without_content into the test names and just make these selector names a.
|
Also, please don't mark a PR as "skip dart-sass" unless it's paired with a Dart Sass PR that's running against this PR. Otherwise, master will go red once this lands. I've removed the "skip" line in the description and re-run the tests. |
See sass/libsass#2842
[skip libsass]