-
Notifications
You must be signed in to change notification settings - Fork 88
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
Test all output styles #270
Conversation
c4c2d4e
to
531680b
Compare
Could you make sure the |
/cc @chriseppstein |
By the way, quite a few todo tests emit this warning in ruby sass:
I wonder if we want to update those tests and if we even want to implement the deprecated syntax? |
We should continue to support those specs until Ruby Sass starts to throw an error. Other wise we're not a drop in replacement. |
Also fixes #247 |
Added PR to my own branch for sass/libsass#922 => mgreter#1 😜 |
IMO we should get a decision if we want to merge this PR before releasing libsass 3.2! |
IMO this is definitely worth doing, especially if all these specs are currently passing on 3.2! As long the utility methods (nuke, unexpected-pass) function correctly then this gets a 👍 from me. The one possible concern could the significantly increased running time of Ruby Sass if they were to switch to sass-spec sometime in the future? It's worth noting it's likely that sass-spec will undergo significant changes with @chriseppstein's work as per #82 (comment). |
👍 This (breaking) change will be the perfect candidate for forthcoming major version of node-sass. //cc @jedfoster. |
IMO the nuke option etc. should still work, since it only adds more tests with different output options and expected output path. But I actually never did any ruby before these changes to the spec runner. So I really would like to get a O.K. from someone that knows ruby. The running time to run the spec test might be an issue, but IMO it's worth it. My perl-libsass spec runner needs 15s-20s to do all 4044 checks (including todo test). Regenerating all 4044 spec test via ruby sass takes about 3 minutes, while exhausting all my 4 cores (i7-3770). It will probably also increase the coverage by one or two percent (I would guess)! |
They contain a lot of edge cases in a pretty brute-force format. Very helpful to discover subtle differences between ruby sass and libsass. Mostly tests interpolation and specially escaped sequences and other format issues.
Reverts the test to the old behavior, where white- space is completely stripped down and not compared.
The are probably hand written and were never generated. They failed with the new more white space sensitive test.
We now test all four output styles with strict white-space rules. We only normalize multiple line-feeds, which we hope to solve with one of the next libsass releases!
A few tests exhibit very strange minimal differences.
60b9a1e
to
8b2245b
Compare
Do a much more white-space sensitive compare. Does only need to normalize multiple line-feeds.
Test all output styles (more white-space sensitive)
Related to sass/libsass#910 and sass/sassc#97