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

Tests failing -- phantomjs versionitis? #8

Closed
ronen opened this issue Dec 5, 2016 · 7 comments · Fixed by #9
Closed

Tests failing -- phantomjs versionitis? #8

ronen opened this issue Dec 5, 2016 · 7 comments · Fixed by #9

Comments

@ronen
Copy link
Contributor

ronen commented Dec 5, 2016

Hi, I wanted to submit a PR or two, and first tried to verify that the tests pass. But on my platform I'm getting some failures:

$ git merge upstream/master
Already up-to-date.
ronen-macbook-pro➜ riot-grid2.git:(master)  riot-grid2 $ npm test

> riot-grid2@3.0.0 test /Users/ronen/github/riot-grid2
> karma start

(node:28220) DeprecationWarning: os.tmpDir() is deprecated. Use os.tmpdir() instead.
DEPRECATION WARNING: jade was renamed "pug" - the jade parser will be removed in riot@3.0.0!
(node:28220) DeprecationWarning: Using Buffer without `new` will soon stop working. Use `new Buffer()`, or preferably `Buffer.from()`, `Buffer.allocUnsafe()` or `Buffer.alloc()` instead.
05 12 2016 14:04:38.020:INFO [framework.browserify]: bundle built
05 12 2016 14:04:38.031:INFO [karma]: Karma v0.13.22 server started at http://localhost:9876/
05 12 2016 14:04:38.047:INFO [launcher]: Starting browser PhantomJS
05 12 2016 14:04:39.627:INFO [PhantomJS 2.1.1 (Mac OS X 0.0.0)]: Connected on socket K2I4rrlOGklweDesAAAA with id 70805820

  grid2
    ✓ should add grid to the document
    ✓ should load data into the grid
    ✓ should render only enough cells needed
    ✓ should render only enough rows after scrolling
    ✓ should render only enough rows after scrolling (again)
    ✓ should pass events through overlay to grid below
    ✗ should change class to active when cell is clicked
	TypeError: undefined is not an object (evaluating '_this.refs.overlay.scrollLeft') (/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/ba693ffb06f1c0ade76d80862febd900.browserify:10 <- lib/grid2.js:9:0)

PhantomJS 2.1.1 (Mac OS X 0.0.0) grid2 should change class to active when cell is clicked FAILED
	TypeError: undefined is not an object (evaluating '_this.refs.overlay.scrollLeft') (/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/ba693ffb06f1c0ade76d80862febd900.browserify:10 <- lib/grid2.js:9:0)
    ✓ should change class to active when cell is clicked
	TypeError: undefined is not an object (evaluating '_this.refs.overlay.scrollLeft') (/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/ba693ffb06f1c0ade76d80862febd900.browserify:10 <- lib/grid2.js:9:0)

    ✗ should toggle a row if clicked twice with meta key
	TypeError: undefined is not an object (evaluating 'dom.setAttribute') (/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/ba693ffb06f1c0ade76d80862febd900.browserify:2323 <- node_modules/riot/riot.js:244:0)

PhantomJS 2.1.1 (Mac OS X 0.0.0) grid2 should toggle a row if clicked twice with meta key FAILED
	TypeError: undefined is not an object (evaluating 'dom.setAttribute') (/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/ba693ffb06f1c0ade76d80862febd900.browserify:2323 <- node_modules/riot/riot.js:244:0)
    ✓ should call onclick on row when cell is clicked
    ✗ should deselect row if meta-clicked
	AssertionError: expected 0 to equal 4 (/Users/ronen/github/riot-grid2/node_modules/chai/chai.js:210)

PhantomJS 2.1.1 (Mac OS X 0.0.0) grid2 should deselect row if meta-clicked FAILED
	AssertionError: expected 0 to equal 4 (/Users/ronen/github/riot-grid2/node_modules/chai/chai.js:210)
    ✓ should show custom tag

PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 12 of 12 (3 FAILED) (0.551 secs / NaN secs)
TOTAL: 3 FAILED, 9 SUCCESS


1) should change class to active when cell is clicked
     grid2
     TypeError: undefined is not an object (evaluating '_this.refs.overlay.scrollLeft') (http://localhost:9876/absolute/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/ba693ffb06f1c0ade76d80862febd900.browserify?9a00f4b435ddb64846363e589c1b6586aa48f2e2:10)
2) should toggle a row if clicked twice with meta key
     grid2
     TypeError: undefined is not an object (evaluating 'dom.setAttribute') (http://localhost:9876/absolute/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/ba693ffb06f1c0ade76d80862febd900.browserify?9a00f4b435ddb64846363e589c1b6586aa48f2e2:2323)
3) should deselect row if meta-clicked
     grid2
     AssertionError: expected 0 to equal 4 (http://localhost:9876/base/node_modules/chai/chai.js?ab7cf506d9d77c111c878b1e10b7f25348630760:210)


=============================== Coverage summary ===============================
Statements   : 93% ( 186/200 )
Branches     : 82.19% ( 60/73 )
Functions    : 92.31% ( 36/39 )
Lines        : 93% ( 186/200 )
================================================================================
npm ERR! Test failed.  See above for more details.
$

I notice that I'm using PhantomJS 2.1.1, whereas the travis build is using PhantomJS 1.9.8, so that could be the source of the problem?

Unfortunately I can't easily drop down to PhantomJS 1.9.8 as PhantomJS 1.9.x crashes on MacOS Sierra, without any workaround: "Please upgrade to PhantomJS 2. 1.9.x is no longer supported, and known to have many crasher bugs that are fixed in 2 version." -- response to ariya/phantomjs#14558)

Also unfortunately it seems travis-ci doesn't yet provide a clean way of using PhantomJS > 1.9.8. See this comment on travis-ci/travis-ci#3225 for links to the apparent latest way to do it. (I haven't tried it myself though)

@crisward
Copy link
Owner

crisward commented Dec 5, 2016

I'm at work at the moment. However I know these tests passed on my laptop which is using sierra. I'll check which version later when I'm at home and update this.

There is also a npm task called tdd ie npm run tdd - which should run tests on file change in chrome. I did a quick test of that here and there are now failing tests. Not sure, but this could be due to the release of riot 3.0.2.

@crisward
Copy link
Owner

crisward commented Dec 5, 2016

Just tried Riot 3.00 and 3.0.1 and both had same fails. Not sure what's going on... (as they've already passed on travis).

@ronen
Copy link
Contributor Author

ronen commented Dec 5, 2016

Adding to the weirdness...

In fiddling with the versions it seemed like it worked once for me, but then I couldn't repeat it. Dunno if it's that lack of sleep thing, or some randomness in the failure.

Also I find that if I comment out these two passing tests, or move them to the end, then all the tests pass:

it "should render only enough rows after scrolling",->
it "should pass events through overlay to grid below",->

So there seems to be some sort of crosstalk between the tests. I haven't looked into what it can be. Both of those tests contain document.querySelector('[ref=overlay]') I don't know if that's a clue...

@crisward
Copy link
Owner

crisward commented Dec 5, 2016

I'd added a setTimeout before update after the click. This was polluting the next test with an update. I've added the setTimeout to the test too, so now all seem to pass. Unfortunatly it looks like I forgot to run npm run build before my last push so the .js file in lib was actually the previous version.

@crisward
Copy link
Owner

crisward commented Dec 5, 2016

Seems to be still passing... all good.

@ronen
Copy link
Contributor Author

ronen commented Dec 5, 2016

Hmm... npm test still isn't working for me. In fact now the test that you added the setTimeout to is failing also (trace below)! npm run tdd works fine though.

$ git log -1
commit caa6da0d557c577b66a640b4ac6e212e1a572f70
Author: Cris Ward <cris@duodesign.co.uk>
Date:   Mon Dec 5 15:41:28 2016 +0000

    added timeout to test
$ npm run build && npm test

> riot-grid2@3.0.0 build /Users/ronen/github/riot-grid2
> riot src lib --template jade

DEPRECATION WARNING: jade was renamed "pug" - the jade parser will be removed in riot@3.0.0!
src/grid2.tag -> lib/grid2.js

> riot-grid2@3.0.0 test /Users/ronen/github/riot-grid2
> karma start

(node:31965) DeprecationWarning: os.tmpDir() is deprecated. Use os.tmpdir() instead.
DEPRECATION WARNING: jade was renamed "pug" - the jade parser will be removed in riot@3.0.0!
(node:31965) DeprecationWarning: Using Buffer without `new` will soon stop working. Use `new Buffer()`, or preferably `Buffer.from()`, `Buffer.allocUnsafe()` or `Buffer.alloc()` instead.
05 12 2016 18:00:58.597:INFO [framework.browserify]: bundle built
05 12 2016 18:00:58.610:INFO [karma]: Karma v0.13.22 server started at http://localhost:9876/
05 12 2016 18:00:58.620:INFO [launcher]: Starting browser PhantomJS
05 12 2016 18:00:59.863:INFO [PhantomJS 2.1.1 (Mac OS X 0.0.0)]: Connected on socket bBqIEIcsgXUQiYyxAAAA with id 61612709

  grid2
    ✓ should add grid to the document
    ✓ should load data into the grid
    ✓ should render only enough cells needed
    ✓ should render only enough rows after scrolling
    ✓ should render only enough rows after scrolling (again)
    ✗ should pass events through overlay to grid below
	TypeError: undefined is not an object (evaluating '_this.refs.overlay.scrollLeft') (/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/a300c7bb6f198a962ef85986b093fe5a.browserify:10 <- lib/grid2.js:9:0)

PhantomJS 2.1.1 (Mac OS X 0.0.0) grid2 should pass events through overlay to grid below FAILED
	TypeError: undefined is not an object (evaluating '_this.refs.overlay.scrollLeft') (/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/a300c7bb6f198a962ef85986b093fe5a.browserify:10 <- lib/grid2.js:9:0)
    ✓ should pass events through overlay to grid below
	TypeError: undefined is not an object (evaluating '_this.refs.overlay.scrollLeft') (/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/a300c7bb6f198a962ef85986b093fe5a.browserify:10 <- lib/grid2.js:9:0)

    ✗ should only select one row at a time without meta key
	TypeError: undefined is not an object (evaluating 'dom.setAttribute') (/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/a300c7bb6f198a962ef85986b093fe5a.browserify:2324 <- node_modules/riot/riot.js:245:0)

PhantomJS 2.1.1 (Mac OS X 0.0.0) grid2 should only select one row at a time without meta key FAILED
	TypeError: undefined is not an object (evaluating 'dom.setAttribute') (/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/a300c7bb6f198a962ef85986b093fe5a.browserify:2324 <- node_modules/riot/riot.js:245:0)
    ✗ should toggle a row if clicked twice with meta key
	AssertionError: expected 0 to equal 4 (/Users/ronen/github/riot-grid2/node_modules/chai/chai.js:210)

PhantomJS 2.1.1 (Mac OS X 0.0.0) grid2 should toggle a row if clicked twice with meta key FAILED
	AssertionError: expected 0 to equal 4 (/Users/ronen/github/riot-grid2/node_modules/chai/chai.js:210)
    ✓ should call onclick on row when cell is clicked
    ✗ should deselect row if meta-clicked
	AssertionError: expected 0 to equal 4 (/Users/ronen/github/riot-grid2/node_modules/chai/chai.js:210)

PhantomJS 2.1.1 (Mac OS X 0.0.0) grid2 should deselect row if meta-clicked FAILED
	AssertionError: expected 0 to equal 4 (/Users/ronen/github/riot-grid2/node_modules/chai/chai.js:210)
    ✓ should show custom tag

PhantomJS 2.1.1 (Mac OS X 0.0.0): Executed 12 of 12 (4 FAILED) (0.593 secs / NaN secs)
TOTAL: 4 FAILED, 8 SUCCESS


1) should pass events through overlay to grid below
     grid2
     TypeError: undefined is not an object (evaluating '_this.refs.overlay.scrollLeft') (http://localhost:9876/absolute/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/a300c7bb6f198a962ef85986b093fe5a.browserify?0632e207c311df8d6318a7899888902a1544884d:10)
2) should only select one row at a time without meta key
     grid2
     TypeError: undefined is not an object (evaluating 'dom.setAttribute') (http://localhost:9876/absolute/var/folders/t7/0tts78p572gb8sw97n0lrtkh0000gn/T/a300c7bb6f198a962ef85986b093fe5a.browserify?0632e207c311df8d6318a7899888902a1544884d:2324)
3) should toggle a row if clicked twice with meta key
     grid2
     AssertionError: expected 0 to equal 4 (http://localhost:9876/base/node_modules/chai/chai.js?ab7cf506d9d77c111c878b1e10b7f25348630760:210)
4) should deselect row if meta-clicked
     grid2
     AssertionError: expected 0 to equal 4 (http://localhost:9876/base/node_modules/chai/chai.js?ab7cf506d9d77c111c878b1e10b7f25348630760:210)


=============================== Coverage summary ===============================
Statements   : 93% ( 186/200 )
Branches     : 82.19% ( 60/73 )
Functions    : 92.31% ( 36/39 )
Lines        : 93% ( 186/200 )
================================================================================
npm ERR! Test failed.  See above for more details.
$

@ronen
Copy link
Contributor Author

ronen commented Dec 5, 2016

Further update. npm run tdd only works intermittently.

[...bit of fiddling...]

Adding another setTimeout seems to fix it... asynchrony in the scroll event?

  it "should render only enough rows after scrolling", (done)->
    document.querySelector('[ref=overlay]').scrollTop = 1000
    setTimeout =>
        expect(document.querySelectorAll('.cell').length).to.be.lt((gridheight/40)*4)
        expect(document.querySelectorAll('.cell').length).to.be.gt((gridheight/40)*3)
        done()

ronen added a commit to ronen/riot-grid2 that referenced this issue Dec 6, 2016
For consistency & completeness, and to forestall future timing issues even though it currently seems OK on tested platforms.

Addresses crisward#8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants