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

scheduleRevalidate in canary is causing test failures #13846

Closed
backspace opened this issue Jul 19, 2016 · 21 comments
Closed

scheduleRevalidate in canary is causing test failures #13846

backspace opened this issue Jul 19, 2016 · 21 comments
Labels
Milestone

Comments

@backspace
Copy link

Hello! travis-web has recently started failing to build on canary. One of the errors is this:

Promise rejected after visiting /builds/branches-tab: null is not an object (evaluating 'node.ownerNode.emberView.scheduleRevalidate')

Which looks to be from this line in ember-htmlbars/lib/utils/subscribe.js:

node.ownerNode.emberView.scheduleRevalidate(node, labelFor(stream));

Looking at the history of that file, I don’t see any reason why recent changes would have produced this failure, but I’m wholly unfamiliar with this code.

Hopefully this is a helpful error report and not just a red herring. Let me know if you need any more information or if maybe it’s a problem in travis-web.

backspace added a commit to backspace/ember.js that referenced this issue Jul 20, 2016
This is an attempt to diagnose this issue:
emberjs#13846
@Serabe Serabe added the Bug label Jul 20, 2016
@Serabe
Copy link
Member

Serabe commented Jul 20, 2016

Thank you for reporting this. I can confirm that this is failing and can be found in emberx-select (though canary is allowed to fail and therefore it was not noticed before). In that line:

  • node is a PropertyAttrMorph. To be exact, a selected from an x-option.
  • node.ownerNode is an EmberMorph whose contextualElement is div#ember-testing.

backspace added a commit to backspace/ember that referenced this issue Jul 20, 2016
This is an attempt to diagnose this issue:
emberjs/ember.js#13846
backspace added a commit to travis-ci/travis-web that referenced this issue Jul 20, 2016
@backspace
Copy link
Author

Thanks for the confirmation, @Serabe. I forked the repository for the built version of Ember and made a commit that adds guards against the absence of node.ownerNode.emberView. I changed travis-web’s ember-try configuration to use that fork as the Bower dependency and the tests now pass with this modified version of canary.

I don’t know enough about this to know whether that type of guard is truly desirable or perhaps a deeper fix where node.ownerNode.emberView should always be present is more the issue.

@rwjblue
Copy link
Member

rwjblue commented Jul 20, 2016

Can someone try to bisect and narrow down which change caused the issue?

@backspace
Copy link
Author

I can try that tomorrow, @rwjblue. Is there an easier way than putting a local filesystem path in bower.json and repeatedly running npm run build during the bisection?

@rwjblue
Copy link
Member

rwjblue commented Jul 21, 2016

@backspace - Yes, definitely.

Something like this:

# in one terminal
git clone [email protected]:components/ember.git components-ember
cd components-ember
git checkout canary
bower link

# in the app
bower link ember
ember serve # you will need to restart this after each bisect stage

Now you can git bisect in the components ember repo (though the SHA's are slightly different). But at least this way you will not have to nom after every bisect step.

@backspace
Copy link
Author

Thanks for the suggestion, looks like it’ll be much faster. I tried it and it said

Bisecting: a merge base must be tested

and then chose a commit from 2014. I think I’ve only used bisect once before, I’ll try to figure out what’s wrong tomorrow.

@backspace
Copy link
Author

backspace commented Jul 21, 2016

Never mind, I think the problem was that I ran git bisect good canary instead of a particular SHA.

19d69b45a106d6e555c9f75db6354495635b34bf is the first bad commit

I tried making a build set to use that commit and the one before it but I had Bower trouble and I should really not be working right now haha

(Edited to add: I got it going. build with last good commit and build with first bad one)

Let me know if I can provide any more diagnostic information.

@rwjblue
Copy link
Member

rwjblue commented Jul 21, 2016

OK, based on the bisect it looks like #13775 introduced the failure.

@rwjblue
Copy link
Member

rwjblue commented Jul 21, 2016

/cc @krisselden

@Serabe
Copy link
Member

Serabe commented Jul 21, 2016

@rwjblue Thank you for bisecting this!

@rwjblue
Copy link
Member

rwjblue commented Jul 21, 2016

Yes, thanks to @backspace for that. I just tracked down which Ember SHA matched the components/ember one he mentioned above.

@dustyjewett
Copy link

Any movement on this? It is now making our ember-beta builds fail.

@xdumaine
Copy link

+1

@backspace
Copy link
Author

Maybe this thread should be locked to contributors to stem the inevitable tide of +1s?

@cibernox
Copy link
Contributor

cibernox commented Aug 4, 2016

I bisected this myself for issue #14000 and also reached the conclusion that #13775 introduced this failure.

@Serabe
Copy link
Member

Serabe commented Aug 16, 2016

Ember 2.8 is already beta 3 and this affects so far:

  • travis-web
  • ember-power-select
  • ember-inspector

I've tried to fix this but is not simple and way above my level. Unless the fix is just guarding against null.

@Serabe
Copy link
Member

Serabe commented Aug 16, 2016

Forgot emberx-select 😞

@pixelhandler
Copy link
Contributor

So this could be a blocker for 2.8.0 release.

@rwjblue
Copy link
Member

rwjblue commented Aug 19, 2016

Yep, definitely is, and I labeled as such last night as I was poking at it a bit...

@miguelcobain
Copy link
Contributor

Trying to help here.

I'm experiencing this bug on PhantomJS but not on Chrome.
Relevant log: https://travis-ci.org/miguelcobain/ember-leaflet/jobs/153965128

@rwjblue
Copy link
Member

rwjblue commented Aug 22, 2016

I have submitted #14110 to resolve this, I would really appreciate anyone having the issue to test out the builds I linked to in that PR.

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

No branches or pull requests

8 participants