node-sass icon indicating copy to clipboard operation
node-sass copied to clipboard

node-sass CLI standard output is a mess

Open saper opened this issue 6 years ago • 1 comments

This is about node-sass master (8d0accabd61ee5cb16248474f2989209b6200e50)

m.saper.info> node -v
v12.11.1
m.saper.info> npm -v
6.11.3

I have re-enabled one watcher test with this diff:

diff --git a/test/cli.js b/test/cli.js
index 78a80910c..dad1e1951 100644
--- a/test/cli.js
+++ b/test/cli.js
@@ -292,7 +292,7 @@ describe('cli', function() {
       }, 500);
     });
 
-    it.skip('should watch the full scss dep tree for a single file (scss)', function(done) {
+    it('should watch the full scss dep tree for a single file (scss)', function(done) {
       var src = fixture('watching/index.scss');
       var foo = fixture('watching/white.scss');
 

And the test fails, probably due to additional logging output moved from stderr to stdout:

  1) cli node-sass in.scss should watch the full scss dep tree for a single file (scss):

      Uncaught AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ '=> changed: /home/saper/node_modules/node-sass/test/fixtures/watching/white.scss'
- 'body{background:blue}'
      + expected - actual

      -=> changed: /home/saper/node_modules/node-sass/test/fixtures/watching/white.scss
      +body{background:blue}
      
      at Socket.<anonymous> (test/cli.js:308:16)
      at addChunk (_stream_readable.js:308:12)
      at readableAddChunk (_stream_readable.js:285:13)
      at Socket.Readable.push (_stream_readable.js:223:10)
      at Pipe.onStreamRead (internal/stream_base_commons.js:182:23)


Of course this can be avoided by adding --quiet but I believe it is not right. CSS should go to standard output, diagnostic, error etc. messages should go to standard error.

Now let's add --quiet:

diff --git a/test/cli.js b/test/cli.js
index 78a80910c..33211e644 100644
--- a/test/cli.js
+++ b/test/cli.js
@@ -292,7 +292,7 @@ describe('cli', function() {
       }, 500);
     });
 
-    it.skip('should watch the full scss dep tree for a single file (scss)', function(done) {
+    it('should watch the full scss dep tree for a single file (scss)', function(done) {
       var src = fixture('watching/index.scss');
       var foo = fixture('watching/white.scss');
 
@@ -300,6 +300,7 @@ describe('cli', function() {
 
       var bin = spawn(cli, [
         '--output-style', 'compressed',
+        '--quiet',
         '--watch', src
       ]);
 

The test fails because the output is generated twice:

  1 failing

  1) cli node-sass in.scss should watch the full scss dep tree for a single file (scss):

      Uncaught AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ 'body{background:blue}\nbody{background:blue}'
- 'body{background:blue}'
                        ^
      + expected - actual

      -body{background:blue}
       body{background:blue}
      
      at Socket.<anonymous> (test/cli.js:309:16)
      at addChunk (_stream_readable.js:308:12)
      at readableAddChunk (_stream_readable.js:285:13)
      at Socket.Readable.push (_stream_readable.js:223:10)
      at Pipe.onStreamRead (internal/stream_base_commons.js:182:23)

Possibly regression on https://github.com/sass/node-sass/pull/2268 and/or https://github.com/sass/node-sass/pull/2344

saper avatar Oct 17 '19 20:10 saper

The second issue (body{background:blue}\nbody{background:blue}) is already covered in https://github.com/sass/node-sass/issues/1152, so let's focus here on change notifications caused by #2268

saper avatar Oct 21 '19 07:10 saper