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

Try to recreate problem with properties in unit tests #431

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brunoocasali
Copy link

@brunoocasali brunoocasali commented Sep 5, 2019

Related with #430

@brunoocasali brunoocasali changed the title WIP: Create a failing test Create a failing test Sep 5, 2019
@brunoocasali brunoocasali changed the title Create a failing test Try to recreate problem with properties in unit tests Sep 5, 2019
@@ -39,7 +39,13 @@ describe('setupRenderingTest', function() {
expect(this.element.textContent.trim()).to.equal('Pretty Color: green');
});

it('renders a second time without', async function() {
it('renders with new color with same property', async function() {
this.set('name', 'red');
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brunoocasali what happens if you change this line to this.name = 'red'? I'm investigating a ton of failures in our app since bumping to ember-source@3.13 and am curious if this is the same issue.

Copy link
Author

@brunoocasali brunoocasali Sep 23, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@efx at my company app using the way you told me worked for some of failing specs, but for some reason in others it doesn't. But I can't reproduce this remaining failing specs in the demo project.

From my company app I have this failing examples: https://gist.github.com/brunoocasali/a50170c59a35e75e7bcaaf078f4cff99
Breaks the last of specs and the first...

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting; thanks for testing that @brunoocasali. I copied your work here and can reproduce the same failure when using this.set('name', 'red') versus this.name = 'red'. I'll raise this in the testing discord to see if anyone has experienced this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! :)
Do you have any ideas about where is the problem origin? Like another lib for example?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some initial bisecting and saw this ember-source PR: emberjs/ember.js#18314 but am not familiar enough to say whether or not it is related. That said, downgrading ember-source does make the tests pass: https://github.com/emberjs/ember-mocha/compare/master...efx:failing-setter-test-downgrade-ember?expand=1
So it is something introduced in 3.13.x

@ghost ghost mentioned this pull request Sep 30, 2019
@ghost ghost mentioned this pull request Oct 15, 2019
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 this pull request may close these issues.

1 participant