Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

node-sass CLI standard output is a mess #2755

Open
saper opened this issue Oct 17, 2019 · 1 comment
Open

node-sass CLI standard output is a mess #2755

saper opened this issue Oct 17, 2019 · 1 comment

Comments

@saper
Copy link
Member

saper commented Oct 17, 2019

This is about node-sass master (8d0acca)

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 #2268 and/or #2344

@saper
Copy link
Member Author

saper commented Oct 21, 2019

The second issue (body{background:blue}\nbody{background:blue}) is already covered in #1152, so let's focus here on change notifications caused by #2268

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant