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

sometimes CSS ordering fails #29

Open
jonathanong opened this issue Mar 30, 2014 · 7 comments
Open

sometimes CSS ordering fails #29

jonathanong opened this issue Mar 30, 2014 · 7 comments

Comments

@jonathanong
Copy link
Contributor

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

@timaschew
Copy link
Member

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
Copy link
Contributor

@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
Copy link
Contributor Author

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

@clintwood
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Member

@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
Copy link
Member

temporary fix via componentjs/resolver.js@983c1fe

timaschew added a commit that referenced this issue Oct 5, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants