Tests fail on Windows #14

XhmikosR · 2014-07-05T07:31:05Z


> critical@0.1.3 test C:\Users\xmr\Desktop\critical
> mocha test.js --timeout 15000

\
....|

  2 passing (2s)
  6 failing

  1)  generates critical-path CSS successfully:
     Uncaught AssertionError: false == true
      at C:\Users\xmr\Desktop\critical\test.js:33:9
      at C:\Users\xmr\Desktop\critical\index.js:60:15
      at C:\Users\xmr\Desktop\critical\node_modules\mocha\node_modules\glob\node_modules\graceful-fs\graceful-fs.js:104:5
      at Object.oncomplete (fs.js:107:15)

  2)  generates minified critical-path CSS successfully:
     Uncaught AssertionError: false == true
      at C:\Users\xmr\Desktop\critical\test.js:47:9
      at C:\Users\xmr\Desktop\critical\index.js:63:13
      at ChildProcess.<anonymous> (C:\Users\xmr\Desktop\critical\node_modules\penthouse\lib\index.js:43:13)
      at ChildProcess.emit (events.js:98:17)
      at Process.ChildProcess._handle.onexit (child_process.js:809:12)

  3)  generates critical-path CSS without writing to disk:
     Uncaught AssertionError: false == true
      at C:\Users\xmr\Desktop\critical\test.js:60:9
      at C:\Users\xmr\Desktop\critical\index.js:63:13
      at ChildProcess.<anonymous> (C:\Users\xmr\Desktop\critical\node_modules\penthouse\lib\index.js:43:13)
      at ChildProcess.emit (events.js:98:17)
      at Process.ChildProcess._handle.onexit (child_process.js:809:12)

  4)  inlines critical-path CSS successfully:
     Uncaught AssertionError: false == true
      at C:\Users\xmr\Desktop\critical\test.js:72:9
      at C:\Users\xmr\Desktop\critical\index.js:92:9
      at C:\Users\xmr\Desktop\critical\node_modules\mocha\node_modules\glob\node_modules\graceful-fs\graceful-fs.js:104:5
      at Object.oncomplete (fs.js:107:15)

  5)  inlines critical-path CSS without writing to disk:
     Uncaught AssertionError: false == true
      at C:\Users\xmr\Desktop\critical\test.js:83:9
      at C:\Users\xmr\Desktop\critical\index.js:95:9
      at fs.js:271:14
      at C:\Users\xmr\Desktop\critical\node_modules\mocha\node_modules\glob\node_modules\graceful-fs\graceful-fs.js:104:5
      at Object.oncomplete (fs.js:107:15)

  6)  inlines and minified critical-path CSS:
     Uncaught AssertionError: false == true
      at C:\Users\xmr\Desktop\critical\test.js:96:9
      at C:\Users\xmr\Desktop\critical\index.js:92:9
      at C:\Users\xmr\Desktop\critical\node_modules\mocha\node_modules\glob\node_modules\graceful-fs\graceful-fs.js:104:5
      at Object.oncomplete (fs.js:107:15)



npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0

I managed to fix one of the tests by forcing LF. 5 still fail though.

The text was updated successfully, but these errors were encountered:

addyosmani · 2014-07-05T09:40:36Z

Hmm. I haven't tested on Windows yet but this looks very curious.

Could you let me know if you're able to get this sample using Critical running for you correctly on Windows https://github.com/addyosmani/critical-path-css-demo?

Alternatively, just verifying the module works in your environment would be of great help. I'm trying to determine if this is a test harness issue, fs, penthouse or straight-out something silly I'm doing in the module.

XhmikosR · 2014-07-05T10:20:29Z

Sure thing. I'll test things out when I'm back.
On Jul 5, 2014 12:40 PM, "Addy Osmani" notifications@github.com wrote:

Hmm. I haven't tested on Windows yet but this looks very curious.

Could you let me know if you're able to get this sample using Critical
running for you correctly on Windows
https://github.com/addyosmani/critical-path-css-demo?

Alternatively, just verifying the module works in your environment would
be of great help. I'm trying to determine if this is a test harness issue,
fs, penthouse or straight-out something silly I'm doing in the module.

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

addyosmani · 2014-07-05T10:57:26Z

Appreciate it!

XhmikosR · 2014-07-05T12:36:57Z

I can't even install the dependencies for the demo... too many errors.

At first I thought it's because of too long filenames. I even moved the repo in C:\ but I still can't install the deps.

XhmikosR · 2014-07-05T12:45:03Z

Full log for the demo https://gist.githubusercontent.com/XhmikosR/2ce380c87e80a8de980f/raw/critical-demo-npm-install.log

addyosmani · 2014-07-05T13:54:41Z

That's crazy. I wonder if the same issues occur with running https://github.com/pocketjoso/penthouse standalone vs. my module. I don't have a windows box handy but will try to debug this issue next week and get to the bottom of it.

XhmikosR · 2014-07-05T15:27:35Z

Nope, penthouse works fine by doing npm install && npm test...

addyosmani · 2014-07-06T12:25:31Z

Interesting. There's something silly I'm doing in this module then. Looking into it.

addyosmani · 2014-07-06T23:29:45Z

@XhmikosR pushed some potential fixes to master. Could you let me know if there's any difference in test output?

XhmikosR · 2014-07-06T23:53:17Z

Sure, I'll try when I'm back☺
On Jul 7, 2014 2:29 AM, "Addy Osmani" notifications@github.com wrote:

@XhmikosR https://github.com/XhmikosR pushed some potential fixes to
master. Could you let me know if there's any difference in test output?

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

XhmikosR · 2014-07-07T01:45:20Z

C:\Users\xmr\Desktop\critical>npm test

> critical@0.1.3 test C:\Users\xmr\Desktop\critical
> mocha test.js --timeout 15000

|
.././

  2 passing (3s)
  6 failing

  1)  generates critical-path CSS successfully:
     Uncaught AssertionError: false == true
      at C:\Users\xmr\Desktop\critical\test.js:33:9
      at C:\Users\xmr\Desktop\critical\index.js:60:15
      at C:\Users\xmr\Desktop\critical\node_modules\mocha\node_modules\glob\node_modules\graceful-fs\graceful-fs.js:104:5
      at Object.oncomplete (fs.js:107:15)

  2)  generates minified critical-path CSS successfully:
     Uncaught AssertionError: false == true
      at C:\Users\xmr\Desktop\critical\test.js:47:9
      at C:\Users\xmr\Desktop\critical\index.js:63:13
      at ChildProcess.<anonymous> (C:\Users\xmr\Desktop\critical\node_modules\penthouse\lib\index.js:43:13)
      at ChildProcess.emit (events.js:98:17)
      at Process.ChildProcess._handle.onexit (child_process.js:809:12)

  3)  generates critical-path CSS without writing to disk:
     Uncaught AssertionError: false == true
      at C:\Users\xmr\Desktop\critical\test.js:60:9
      at C:\Users\xmr\Desktop\critical\index.js:63:13
      at ChildProcess.<anonymous> (C:\Users\xmr\Desktop\critical\node_modules\penthouse\lib\index.js:43:13)
      at ChildProcess.emit (events.js:98:17)
      at Process.ChildProcess._handle.onexit (child_process.js:809:12)

  4)  inlines critical-path CSS successfully:
     Uncaught AssertionError: false == true
      at C:\Users\xmr\Desktop\critical\test.js:72:7
      at C:\Users\xmr\Desktop\critical\index.js:92:9
      at C:\Users\xmr\Desktop\critical\node_modules\mocha\node_modules\glob\node_modules\graceful-fs\graceful-fs.js:104:5
      at Object.oncomplete (fs.js:107:15)

  5)  inlines critical-path CSS without writing to disk:
     Uncaught AssertionError: false == true-
  6)  inlines and minified critical-path CSS:s:83:7
     Uncaught AssertionError: false == truex.js:95:9
      at C:\Users\xmr\Desktop\critical\test.js:96:7
      at C:\Users\xmr\Desktop\critical\index.js:92:9mocha\node_modules\glob\node
      at C:\Users\xmr\Desktop\critical\node_modules\mocha\node_modules\glob\node_modules\graceful-fs\graceful-fs.js:104:5
      at Object.oncomplete (fs.js:107:15)



npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0

addyosmani · 2014-07-08T20:58:48Z

Thanks to Sindre we have a fresh rewrite now in master. Any luck now?

XhmikosR · 2014-07-08T21:03:41Z

I tried a couple hours ago after those patches and I still get the same
failures...
On Jul 8, 2014 11:58 PM, "Addy Osmani" notifications@github.com wrote:

Thanks to Sindre we have a fresh rewrite now in master. Any luck now?

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

addyosmani · 2014-07-08T21:15:51Z

This is really weird. @sindresorhus I don't suppose you have any hints that would be worth considering here, would you?

sindresorhus · 2014-07-08T21:54:42Z

@XhmikosR run the test from master now and paste in the output ;)

Also look at the terminal output from installing dependencies. The phantomjs install might have failed.

XhmikosR · 2014-07-08T22:59:25Z

Sure, I'll do it first thing tomorrow.

On Wed, Jul 9, 2014 at 12:54 AM, Sindre Sorhus notifications@github.com
wrote:

@XhmikosR https://github.com/XhmikosR run the test from master now and
paste in the output ;)

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

XhmikosR · 2014-07-09T07:32:37Z

https://gist.github.com/XhmikosR/8b69a2a39841f9edfd05

XhmikosR · 2014-07-09T07:33:04Z

With LF forced for the test files, I get 3/6 passed BTW.

addyosmani · 2014-07-11T14:15:54Z

I wonder if it's more sensible for our API tests to just test against the output of each method directly rather than writing to disk and comparing the read-in input. Do any of our non i/o tests pass on Windows?

XhmikosR · 2014-07-12T06:16:47Z

With this patch and LF forced for all the repository files I get 3 failing tests.


C:\Users\xmr\Desktop\critical>npm test



> critical@0.1.4 test C:\Users\xmr\Desktop\critical
> mocha test.js --timeout 15000

|
..../

  5 passing (44s)
  3 failing

  1)  generates critical-path CSS successfully:
     Error: timeout of 15000ms exceeded
      at null.<anonymous> (C:\Users\xmr\Desktop\critical\node_modules\mocha\lib\runnable.js:139:19)
      at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)

  2)  generates minified critical-path CSS successfully:
     Error: timeout of 15000ms exceeded
      at null.<anonymous> (C:\Users\xmr\Desktop\critical\node_modules\mocha\lib\runnable.js:139:19)
      at Timer.listOnTimeout [as ontimeout] (timers.js:110:15)

  3)  generates critical-path CSS without writing to disk:

      Uncaught AssertionError: "body {\n    padding-top: 20px;\n    padding-bott
om: 20px;\n}\n\n\n.header,\n.marketing{\n    padding-left: 15px;\n    padding-r
=== "body {\r\n    padding-top: 20px;\r\n    padding-bottom: 20px;\r\n}\r\n\r\n\
r\n.header,\r\n.marketing{\r\n    padding-left: 15px
      + expected - actual

      +body {<CR>
      -body {
-    padding-top: 20px;
      -    padding-bottom: 20px;
      -}

      +    padding-top: 20px;<CR>

      +    padding-bottom: 20px;<CR>
      -.header,
      -.marketing{
      -    padding-left: 15px;
      -    padding-right: 15px;
      -}

      +}<CR>

      +<CR>
      -.header {
      -    border-bottom: 1px solid #e5e5e5;
      -}

      +<CR>

      +.header,<CR>
      -.header h3 {
      -    margin-top: 0;
      -    margin-bottom: 0;
      /-    line-height: 40px;
      -    padding-bottom: 19px;
      -}

      +.marketing{<CR>

      +    padding-left: 15px;<CR>
      -.jumbotron {
      -    text-align: center;
      -    border-bottom: 1px solid #e5e5e5;
      -}

      +    padding-right: 15px;<CR>
      -.jumbotron .btn {
      -    font-size: 21px;
      -    padding: 14px 24px;
      -}

      +}<CR>

      +<CR>
      -.marketing {
      -    margin: 40px 0;
      -}

      +<CR>

      +.header {<CR>
      -@media screen and (min-width: 768px) {
      -    .container {
      -        max-width: 730px;
      -    }

      +    border-bottom: 1px solid #e5e5e5;<CR>
      -
      -    .header,
      -    .marketing{
      -        padding-left: 0;
      -        padding-right: 0;
      -    }

      +}<CR>
      +<CR>
      +<CR>
+    margin-top: 0;<CR>-
      +    margin-bottom: 0;<CR>
      +    line-height: 40px;<CR>
      +    padding-bottom: 19px;<CR>
      +}<CR>
      +<CR>
      +<CR>
      +.jumbotron {<CR>
      +    text-align: center;<CR>
      +    border-bottom: 1px solid #e5e5e5;<CR>
      +}<CR>
      +<CR>
      +.jumbotron .btn {<CR>
      +    font-size: 21px;<CR>
      +    padding: 14px 24px;<CR>
      +}<CR>
      +<CR>
      +<CR>
      +.marketing {<CR>
      +    margin: 40px 0;<CR>
      +}<CR>
      +<CR>
      +<CR>
      +@media screen and (min-width: 768px) {<CR>
      +    .container {<CR>
      +        max-width: 730px;<CR>
      +    }<CR>
      \+<CR>
      +    <CR>
      +    .header,<CR>
      +    .marketing{<CR>
      +        padding-left: 0;<CR>
      +        padding-right: 0;<CR>
      +    }<CR>
      +<CR>
      +    <CR>
      +    .header {<CR>
      +        margin-bottom: 30px;<CR>
      +    }<CR>
      +    <CR>
      +    <CR>
      +    .jumbotron {<CR>
      +        border-bottom: 0;<CR>
      +    }<CR>
      +}<CR>
      -
      -    .header {
      -        margin-bottom: 30px;
      -    }
      -
      -
      -    .jumbotron {
      -        border-bottom: 0;
      -    }
      -}

      at C:\Users\xmr\Desktop\critical\test.js:28:16
      at C:\Users\xmr\Desktop\critical\index.js:75:21
      at C:\Users\xmr\Desktop\critical\node_modules\mocha\node_modules\glob\node_modules\graceful-fs\graceful-fs.js:104:5
      at Object.oncomplete (fs.js:107:15)



npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0

So I'd say we should have this merged.

addyosmani · 2014-07-13T18:52:03Z

I've been testing the module on Windows 8 today and ran into the same issues @XhmikosR mentions above. In addition, even if we're not just talking about the unit tests, the demo app using this module also seems to silently fail (builds, but critical path CSS is not inlined).

Something like this however does work (pardon Notepad):

In contrast, I was able to get the penthouse test working locally on windows with no issues.

addyosmani · 2014-07-13T20:51:26Z

Working on fixes. Have 4 tests passing now. Just a few more to go :)

XhmikosR · 2014-07-14T14:25:45Z

Cool, almost there 3/4 passing. One thing I notice with the tests branch is
that fixture\styles\critical.css is modified and is different so that must
be the one failing test.

Also the isWindows variable seem to be only used once, it's your call if
you want to keep it. var lineBreak = process.platform == 'win32' ? /\r\n/g : /\n/g; would work too.

On Sun, Jul 13, 2014 at 11:51 PM, Addy Osmani notifications@github.com
wrote:

Working on fixes. Have 4 tests passing now. Just a few more to go :)

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

addyosmani · 2014-07-14T14:49:19Z

Good catch. We could indeed just simplify that further. I think you're right that the critical.css file might just be the one that needs correcting. Most of the tests are failing on inlining atm.

sindresorhus · 2014-07-14T15:19:25Z

var lineBreak = process.platform == 'win32' ? /\r\n/g : /\n/g; would work too.

You want require('os').EOL.

addyosmani · 2014-07-14T15:28:37Z

Hah! I didn't realize that was supported :)

XhmikosR · 2014-07-14T15:36:18Z

Didn't know this existed either, thanks.

bezoerb · 2014-11-25T22:28:07Z

All green now on windows even though the newlines had to be fixed in inline-critical

bezoerb · 2015-02-02T22:46:41Z

Tests still green on windows. Closing this for now.

addyosmani added the bug label Jul 5, 2014

bezoerb added a commit that referenced this issue Nov 25, 2014

Updated version of inline-critical to address #14

a6e1882

bezoerb added a commit that referenced this issue Nov 25, 2014

Fixed newline character in css to address #14

c343133

bezoerb closed this as completed Feb 2, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Tests fail on Windows #14

Tests fail on Windows #14

XhmikosR commented Jul 5, 2014

addyosmani commented Jul 5, 2014

XhmikosR commented Jul 5, 2014

addyosmani commented Jul 5, 2014

XhmikosR commented Jul 5, 2014

XhmikosR commented Jul 5, 2014

addyosmani commented Jul 5, 2014

XhmikosR commented Jul 5, 2014

addyosmani commented Jul 6, 2014

addyosmani commented Jul 6, 2014

XhmikosR commented Jul 6, 2014

XhmikosR commented Jul 7, 2014

addyosmani commented Jul 8, 2014

XhmikosR commented Jul 8, 2014

addyosmani commented Jul 8, 2014

sindresorhus commented Jul 8, 2014

XhmikosR commented Jul 8, 2014

XhmikosR commented Jul 9, 2014

XhmikosR commented Jul 9, 2014

addyosmani commented Jul 11, 2014

XhmikosR commented Jul 12, 2014

addyosmani commented Jul 13, 2014

addyosmani commented Jul 13, 2014

XhmikosR commented Jul 14, 2014

addyosmani commented Jul 14, 2014

sindresorhus commented Jul 14, 2014

addyosmani commented Jul 14, 2014

XhmikosR commented Jul 14, 2014

bezoerb commented Nov 25, 2014

bezoerb commented Feb 2, 2015

Tests fail on Windows #14

Tests fail on Windows #14

Comments

XhmikosR commented Jul 5, 2014

addyosmani commented Jul 5, 2014

XhmikosR commented Jul 5, 2014

addyosmani commented Jul 5, 2014

XhmikosR commented Jul 5, 2014

XhmikosR commented Jul 5, 2014

addyosmani commented Jul 5, 2014

XhmikosR commented Jul 5, 2014

addyosmani commented Jul 6, 2014

addyosmani commented Jul 6, 2014

XhmikosR commented Jul 6, 2014

XhmikosR commented Jul 7, 2014

addyosmani commented Jul 8, 2014

XhmikosR commented Jul 8, 2014

addyosmani commented Jul 8, 2014

sindresorhus commented Jul 8, 2014

XhmikosR commented Jul 8, 2014

XhmikosR commented Jul 9, 2014

XhmikosR commented Jul 9, 2014

addyosmani commented Jul 11, 2014

XhmikosR commented Jul 12, 2014

addyosmani commented Jul 13, 2014

addyosmani commented Jul 13, 2014

XhmikosR commented Jul 14, 2014

addyosmani commented Jul 14, 2014

sindresorhus commented Jul 14, 2014

addyosmani commented Jul 14, 2014

XhmikosR commented Jul 14, 2014

bezoerb commented Nov 25, 2014

bezoerb commented Feb 2, 2015