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

Warning: compact has been removed breaks node-sass coverage #360

Closed
saper opened this issue May 1, 2015 · 12 comments
Closed

Warning: compact has been removed breaks node-sass coverage #360

saper opened this issue May 1, 2015 · 12 comments

Comments

@saper
Copy link
Member

saper commented May 1, 2015

My environment:

(libsass has been compiled separately and linked to the node-sass binding as a shared library)

When running npm test I get the following warning:

      ✓ bool
Warning: `compact` has been removed from libsass because it's not part of the Sass spec
Warning: `compact` has been removed from libsass because it's not part of the Sass spec
Warning: `compact` has been removed from libsass because it's not part of the Sass spec
Warning: `compact` has been removed from libsass because it's not part of the Sass spec
      ✓ bourbon (43ms)
      ✓ calc

but the tests pass. Trying however to run mocha in the coverage testing mode fails immediately:

>  npm run coverage

> node-sass@3.0.0-beta.7 coverage /home/saper/node_modules/node-sass
> node scripts/coverage.js

Warning: `compact` has been removed from libsass because it's not part of the Sass spec


npm ERR! FreeBSD 10.1-STABLE
npm ERR! argv "node" "/usr/local/bin/npm" "run" "coverage"
npm ERR! node v0.12.2
npm ERR! npm  v2.8.4
npm ERR! code ELIFECYCLE
npm ERR! node-sass@3.0.0-beta.7 coverage: `node scripts/coverage.js`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the node-sass@3.0.0-beta.7 coverage script 'node scripts/coverage.js'.
npm ERR! This is most likely a problem with the node-sass package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node scripts/coverage.js
npm ERR! You can get their info via:
npm ERR!     npm owner ls node-sass
npm ERR! There is likely additional logging output above.

npm ERR! Please include the following file with any support request:
npm ERR!     /home/saper/node_modules/node-sass/npm-debug.log

Maybe the warning should be relaxed or compact test files removed from the spec?

Ping sass/node-sass#904 sass/libsass#1055 #359

@xzyfer
Copy link
Contributor

xzyfer commented May 1, 2015

@mgreter this is what I was worried about. Users are now getting warned even if they've defined their own compact function. I'm just going to remove compact for good.

@saper
Copy link
Member Author

saper commented May 1, 2015

Isn't that because tests contain ...compact.css files?

@xzyfer
Copy link
Contributor

xzyfer commented May 1, 2015

No. We originally had a native compact function in Libsass. It was removed in 3.2.0 because it was non-standard. In 3.2.1 we started throwing a warning you tried to used the compact function, however the code doesn't only warning if you're trying to use the native version i.e. if you have defined your own compact function.

@saper
Copy link
Member Author

saper commented May 1, 2015

Why does bool test generate a warning?

@xzyfer
Copy link
Contributor

xzyfer commented May 1, 2015

I believe the error is from the bourbon spec below, which defines it's own compact function.

@xzyfer
Copy link
Contributor

xzyfer commented May 1, 2015

Please try with this patch. sass/libsass#1156

@mgreter
Copy link
Contributor

mgreter commented May 1, 2015

@xzyfer Go for it, but I wonder why this did not happen with official spec tests. I mean bourbon is part of the official spec test suite!? But appologies if the warning causes any problems!??
@saper may it be that mocha does not like any output on STDERR?

@xzyfer
Copy link
Contributor

xzyfer commented May 1, 2015

@mgreter Yeah couldn't reproduce it either.

@sapar I've released libsass 3.2.2 - https://github.com/sass/libsass/releases/tag/3.2.2

@xzyfer
Copy link
Contributor

xzyfer commented May 1, 2015

We knew this was going to happen, didn't expect it to affect node-sass though

sass/libsass#1055 (comment)

@saper
Copy link
Member Author

saper commented May 1, 2015

@xzyfer I have more fixes for sassc so pls hold your horses :)

@xzyfer
Copy link
Contributor

xzyfer commented May 1, 2015

I haven't yet released sassc.
On 1 May 2015 12:09, "Marcin Cieślak" notifications@github.com wrote:

@xzyfer https://github.com/xzyfer I have more fixes for sassc so pls
hold your horses :)


Reply to this email directly or view it on GitHub
#360 (comment).

@saper
Copy link
Member Author

saper commented May 1, 2015

Thanks, it works now!

@saper saper closed this as completed May 1, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants