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.

Sign up for GitHub

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
Open

Try to recreate problem with properties in unit tests #431

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion tests/unit/setup-rendering-test-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 [email protected] and am curious if this is the same issue.

Copy link
Author

@brunoocasali brunoocasali Sep 23, 2019 •
edited
Loading

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

await render(hbs`{{pretty-color name=name}}`);
expect(this.element.textContent.trim()).to.equal('Pretty Color: red');
});

it('renders a third time without', async function() {
await render(hbs`{{pretty-color name=name}}`);
expect(this.element.textContent.trim()).to.equal('Pretty Color:');
});
Expand Down