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.

Sign up for GitHub

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

Jump to bottom

Fix specs on Ruby 2.1.0 #1072

Closed
wants to merge 2 commits into from
Closed

Fix specs on Ruby 2.1.0 #1072

wants to merge 2 commits into from

Conversation

felixbuenemann
Copy link

Fixes #1064. See issue for more info. TL;DR: Ruby 2.1.0 has different order for uniq, which makes some tests fail.

28a9911 is the simplest possible fix I could come up with, but which I found ugly to read. So 47b9249 implements an assert_permutation method to match ignoring class order.
@felixbuenemann
Fix specs that depend on class order
28a9911 
This mkes them pass on ruby 2.1, which returns a different order for
uniq compared to older ruby versions.
@felixbuenemann
Add assert_permutation syntactic suggar
47b9249 
This allows to match selectors against extensions independent of class
order with all possible permutations.

Example:

input:
.abc.def.ghi

permutations:
.abc.def.ghi
.abc.ghi.def
.def.abc.ghi
.def.ghi.abc
.ghi.abc.def
.ghi.def.abc
@felixbuenemann
Copy link
Author

@chriseppstein Choose whichever solution you prefer and I'll squash.

The assert_permutation method should be generic enough to use it in other tests as well. However it is complex enough to warrant its own unit test.
@chriseppstein
Copy link

I toyed around with what it would take to make the Sass AST have a semantic equality check and it got hairy pretty fast.

None of these options make me particularly happy. Ultimately, I think we will find that and AST-based comparison is the most robust way of testing CSS equality -- especially as a way to test the output from other Sass implementations where minor output differences are irrelevant to be considered "compatible". However, for the time being, I think I prefer 28a9911 because it's a small fix even though it's not general.

@nex3 I leave the final decision to you.
@HamptonMakes
Copy link
Member

I've been playing around with this with sass-spec branches. Hard. Hard.
Hard.

On Tue, Jan 14, 2014 at 4:44 PM, Chris Eppstein notifications@github.comwrote:

I toyed around with what it would take to make the Sass AST have a
semantic equality check and it got hairy pretty fast.

None of these options make me particularly happy. Ultimately, I think we
will find that and AST-based comparison is the most robust way of testing
CSS equality -- especially as a way to test the output from other Sass
implementations where minor output differences are irrelevant to be
considered "compatible". However, for the time being, I think I prefer
28a991128a99115b9b6925e22189d3d789ca019effba191because it's a small fix even though it's not general.

@nex3 https://github.com/nex3 I leave the final decision to you.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/1072#issuecomment-32324238
.
@felixbuenemann
Copy link
Author

I believe @nex3 already commented, that he would prefer to wait on the fix to ruby 2.1.0, which should land in the next minor release. I believe in that case ruby 2.1.0 should be marked as "Allowed Failures" on Travis CI for the time being.
@nex3
Copy link
Contributor

nex3 commented Jan 22, 2014

Yeah, I just want to wait for the Ruby 2.1.0 fix. 05fc8e2 marks the tests as failing on 2.1.0 for the time being.
@nex3 nex3 closed this Jan 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants
@felixbuenemann @chriseppstein @HamptonMakes @nex3