componentjs / builder2.js

builder for component
50 stars 20 forks source link

sometimes CSS ordering fails #29

Open jonathanong opened 10 years ago

jonathanong commented 10 years ago

generally only when i first run tests though. afterwards, the error doesn't pop up again. URGHHH

timaschew commented 10 years ago

not only the first time, but in the most cases it's the first time:

 mocha:runner run suite css-local-ordering +3ms
  css-local-ordering
  remotes:local checking local components at /Users/awilhelm/dev/component-builder2.js/components +1ms
  component-resolver remote not set - defaulting to remotes's defaults +0ms
  component-resolver:locals resolving local at "/Users/awilhelm/dev/component-builder2.js/test/fixtures/css-local-ordering" +1ms
  component-resolver resolving "css-local-ordering" +1ms
  component-resolver:locals resolving "css-local-ordering"'s local dependency "one". +0ms
  component-resolver:locals looking up "css-local-ordering"'s local dependency "one" at "/Users/awilhelm/dev/component-builder2.js/test/fixtures/css-local-ordering/lib/one". +0ms
  component-resolver:locals resolving "css-local-ordering"'s local dependency "two". +0ms
  component-resolver:locals looking up "css-local-ordering"'s local dependency "two" at "/Users/awilhelm/dev/component-builder2.js/test/fixtures/css-local-ordering/lib/two". +4ms
  component-resolver remaining dependencies: 0 +0ms
  component-resolver remaining semver: 0 +0ms
  component-resolver:locals resolving local at "/Users/awilhelm/dev/component-builder2.js/test/fixtures/css-local-ordering/lib/one" +0ms
  component-resolver:locals resolving local at "/Users/awilhelm/dev/component-builder2.js/test/fixtures/css-local-ordering/lib/two" +1ms
  component-resolver resolving "two" +1ms
  component-resolver remaining dependencies: 0 +0ms
  component-resolver remaining semver: 0 +0ms
  component-resolver:locals resolved local "css-local-ordering" +0ms
  component-resolver resolving "one" +0ms
  component-resolver remaining dependencies: 0 +0ms
  component-resolver remaining semver: 0 +0ms
  component-resolver:locals resolved local "css-local-ordering" +0ms
  component-resolver finished resolving locals +0ms
  component-resolver finished resolving dependencies (1) +0ms
  component-resolver finished installing dependencies +0ms
    ✓ should install 
    ✓ should build 
    1) should be correct

  mocha:runner run suite css-url-rewriting +4ms
  css-url-rewriting

  mocha:runner run suite css-url-rewriting-locals +0ms
  css-url-rewriting-locals

  mocha:runner run suite css-glob +0ms
  css-glob

  mocha:runner run suite font-awesome +0ms
  font-awesome

  mocha:runner run suite symlink +0ms
  symlink
  mocha:runner run suite symlink destination option +0ms
    destination option

  mocha:runner finished running +0ms

  69 passing (7s)
  1 failing

  1) css-local-ordering should be correct:

      AssertionError: expected '.two {}\n\n.one {}' to be '.one {}\n\n.two {}'
      + expected - actual

      +".one {}\n\n.two {}"
      -".two {}\n\n.one {}"

      at Assertion.prop.(anonymous function) (/Users/awilhelm/dev/component-builder2.js/node_modules/should/lib/should.js:61:14)
      at Context.<anonymous> (/Users/awilhelm/dev/component-builder2.js/test/styles.js:42:25)
      at callFn (/Users/awilhelm/dev/component-builder2.js/node_modules/mocha/lib/runnable.js:249:21)
      at Test.Runnable.run (/Users/awilhelm/dev/component-builder2.js/node_modules/mocha/lib/runnable.js:242:7)
      at Runner.runTest (/Users/awilhelm/dev/component-builder2.js/node_modules/mocha/lib/runner.js:373:10)
      at /Users/awilhelm/dev/component-builder2.js/node_modules/mocha/lib/runner.js:451:12
      at next (/Users/awilhelm/dev/component-builder2.js/node_modules/mocha/lib/runner.js:298:14)
      at /Users/awilhelm/dev/component-builder2.js/node_modules/mocha/lib/runner.js:308:7
      at next (/Users/awilhelm/dev/component-builder2.js/node_modules/mocha/lib/runner.js:246:23)
      at Immediate._onImmediate (/Users/awilhelm/dev/component-builder2.js/node_modules/mocha/lib/runner.js:275:5)
      at processImmediate [as _immediateCallback] (timers.js:374:17)
clintwood commented 10 years ago

@timaschew, @jonathanong, can confirm it is not only on first run, randomly swaps order... frack, this is breaking things all over the place for me... taking a look at it now...

jonathanong commented 10 years ago

haha dude. i couldn't figure it out because it was so hard to replicate. i'll be impressed if you do!

clintwood commented 10 years ago

hahaha, love a challenge!

Ok, good news I know the source of the issue! Bad news I'm not sure how to fix it!

To reproduce: 1) make it more repeatable: add a few more CSS components to the css-local-ordering test. I added two more components named 'three' & 'four'... I'm on windoze and get the error roughly once every five runs. 2) comment out all except css-local-ordering in ./test/styles.js and target just that test...

To see the issue: replace https://github.com/component/resolver.js/blob/master/lib/locals.js#L177 with

    console.log('A: %s', filename)
    buf = yield fs.readFile.bind(null, filename, 'utf8');
    console.log('B: %s', filename)

What you will see is that the order going in at A: ... is consistent but the order coming out at B: ... sometimes changes... I expected the incoming order to be honored... BTW its not graceful-fs I tried fs and still have this issue. Of course readFileSync eliminates the error but that's probably not what we want here...

@jonathanong, since this is possibly somewhere in fs or generator implementation, how do we resolve it?

clintwood commented 10 years ago

Also just checked if file size affects out order and it's not related... using a large CSS file as say component one does not force it down in the order (due to read time)...

timaschew commented 9 years ago

@jonathanong so found out that it works if I set concurrency of the channel to 1 it never happens.

So either the usage of https://github.com/cojs/chanel is wrong or the library has a bug.

timaschew commented 9 years ago

temporary fix via https://github.com/componentjs/resolver.js/commit/983c1feb9ce86630a407d4d7ac3033aacbced823