[GLIMMER2] Moves unbound tests #13137

chadhietala · 2016-03-20T01:56:56Z

This moves the unbound tests to the new test infra and implements the unbound helper.

chadhietala · 2016-03-20T01:57:56Z

packages/ember-glimmer/tests/integration/helpers/unbound-test.js

+    this.assertText('BORK BORK');
+  }
+
+  ['@htmlbars sexpr form does not update on parent re-render']() {


Is it just me or is it weird that sexprs and syntax act differently when the parent re-renders?

Combination of

ember.js/packages/ember-htmlbars/tests/helpers/unbound_test.js

Lines 179 to 196 in bf067f7

QUnit.test('it should render the current value of a property on the context', function() {

equal(view.$().text(), 'BORK', 'should render the current value of a property');

});

QUnit.test('it should not re-render if the property changes', function() {

run(function() {

view.set('context.foo', 'oof');

});

equal(view.$().text(), 'BORK', 'should not re-render if the property changes');

});

QUnit.test('it should not re-render if the parent view rerenders', function() {

run(function() {

view.set('context.foo', 'oof');

view.rerender();

});

equal(view.$().text(), 'BORK', 'should not re-render if the parent view rerenders');

});

chadhietala · 2016-03-20T18:52:53Z

packages/ember-glimmer/lib/environment.js

@@ -50,6 +50,8 @@ export default class extends Environment {
        return new DynamicComponentSyntax({ args, templates });
      } else if (key === 'outlet') {
        return new OutletSyntax({ args });
+      } else if (key === 'partial') {


Opps will remove.

chadhietala · 2016-03-20T18:59:27Z

packages/ember-glimmer/tests/integration/helpers/unbound-test.js

+
+moduleFor('Helpers test: {{unbound}}', class extends RenderingTest {
+
+  ['@htmlbars it should not update the value if the parent view doesn\'t rerender']() {


Combines https://github.com/emberjs/ember.js/blob/master/packages/ember-htmlbars/tests/helpers/unbound_test.js#L45-L54

chadhietala · 2016-03-20T19:15:38Z

Tests that were removed are:

Container lookup tests 1 2

Property With Attributes <--- Maybe bring this one back. This checks for property on an object where as another test just for property on the context.

chadhietala · 2016-03-20T20:15:28Z

@chancancode These are probably G2G given we are ok with the tests that were removed.

chancancode · 2016-03-20T22:58:28Z

packages/ember-htmlbars/tests/helpers/unbound_test.js

-  }
-});
-
-QUnit.test('it should render the current value of a property on the context', function() {


✔️ it should not update the value if the parent view doesn't rerender

chancancode · 2016-03-22T19:12:14Z

packages/ember-htmlbars/tests/helpers/unbound_test.js

-  equal(view.$().text(), 'XXXXX X XX XXXX', 'only unbound bound options changed');
-});
-
-QUnit.test('should be able to render an bound helper invocation mixed with static values', function() {


✔️ should be able to render an bound helper invocation mixed with static values

chancancode · 2016-03-22T19:19:26Z

@chadhietala mostly looks good to me, left some feedback as line comments. I don't love the way the latter, more complicated tests are factored, the setup required are quite distracting and it took me a long time to understand what they do. It's not your fault though, and I think we don't have to worry about those now. Hopefully when everything settles someone will have the appetite to refactor these one day :P

chancancode · 2016-03-22T19:20:29Z

I'm a bit worried about the inconsistent semantics because our new implementation does not "naturally" have those problems, so I'll ask around in #core to figure out what we actually need to implement (w.r.t. rerender)

chancancode · 2016-03-22T19:20:36Z

(Oops)

chadhietala · 2016-03-23T01:52:14Z

I think CI just needs to be restarted. This fell victim to the left-pad debacle.

homu · 2016-03-23T02:17:07Z

☔ The latest upstream changes (presumably #13160) made this pull request unmergeable. Please resolve the merge conflicts.

chancancode · 2016-03-24T18:29:13Z

packages/ember-glimmer/tests/integration/helpers/unbound-test.js

+  ['@test it should not update the value if the parent view doesn\'t rerender']() {
+    this.render(`{{unbound foo}} {{unbound bar}}`, {
+      foo: 'BORK',
+      barBinding: 'foo'


Can ✂️, I think?

chancancode · 2016-03-24T18:41:25Z

Looks great to me! There are a few minor things but they don't really need to block merging. However, I do want to hear from @rwjblue about the owner test before merging.

This moves the unbound tests to the new test infra and integrates `unbound` to Glimmer.

chadhietala reviewed Mar 20, 2016
View reviewed changes

chadhietala force-pushed the glim-glam-unbound branch 3 times, most recently from 5169fd1 to 63f48fd Compare March 20, 2016 18:04

chadhietala reviewed Mar 20, 2016
View reviewed changes

chadhietala force-pushed the glim-glam-unbound branch from 63f48fd to 0d53945 Compare March 20, 2016 18:57

chadhietala reviewed Mar 20, 2016
View reviewed changes

chadhietala force-pushed the glim-glam-unbound branch from 0d53945 to fe038d2 Compare March 20, 2016 19:30

chancancode reviewed Mar 20, 2016
View reviewed changes

chancancode reviewed Mar 22, 2016
View reviewed changes

chancancode closed this Mar 22, 2016

chancancode reopened this Mar 22, 2016

chadhietala force-pushed the glim-glam-unbound branch from b521870 to 621feb1 Compare March 22, 2016 21:40

chadhietala force-pushed the glim-glam-unbound branch 2 times, most recently from 3f5badf to 10f875d Compare March 23, 2016 17:30

chancancode reviewed Mar 24, 2016
View reviewed changes

chadhietala force-pushed the glim-glam-unbound branch from 10f875d to 8154718 Compare March 24, 2016 22:20

[GLIMMER2] Moves unbound tests

940492d

This moves the unbound tests to the new test infra and integrates `unbound` to Glimmer.

chadhietala force-pushed the glim-glam-unbound branch from 8154718 to 940492d Compare March 26, 2016 15:27

chancancode merged commit a62c820 into emberjs:master Mar 26, 2016

homu mentioned this pull request Mar 26, 2016

[Glimmer 2] Implement component lifecycle hooks #13168

Merged

chadhietala deleted the glim-glam-unbound branch March 26, 2016 16:58

chancancode mentioned this pull request Apr 7, 2016

[Pls send halp!] The Ultimate Glimmer 2 Test Porting Guide #13127

Closed

45 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[GLIMMER2] Moves unbound tests #13137

[GLIMMER2] Moves unbound tests #13137

chadhietala commented Mar 20, 2016 •

edited

Loading

chadhietala Mar 20, 2016

chadhietala Mar 20, 2016

chadhietala Mar 20, 2016

chadhietala Mar 20, 2016

chadhietala commented Mar 20, 2016

chadhietala commented Mar 20, 2016

chancancode Mar 20, 2016

chancancode Mar 22, 2016

chancancode commented Mar 22, 2016

chancancode commented Mar 22, 2016

chancancode commented Mar 22, 2016

chadhietala commented Mar 23, 2016

homu commented Mar 23, 2016

chancancode Mar 24, 2016

chancancode commented Mar 24, 2016

	QUnit.test('it should render the current value of a property on the context', function() {
	equal(view.$().text(), 'BORK', 'should render the current value of a property');
	});

	QUnit.test('it should not re-render if the property changes', function() {
	run(function() {
	view.set('context.foo', 'oof');
	});
	equal(view.$().text(), 'BORK', 'should not re-render if the property changes');
	});

	QUnit.test('it should not re-render if the parent view rerenders', function() {
	run(function() {
	view.set('context.foo', 'oof');
	view.rerender();
	});
	equal(view.$().text(), 'BORK', 'should not re-render if the parent view rerenders');
	});


		moduleFor('Helpers test: {{unbound}}', class extends RenderingTest {

		['@htmlbars it should not update the value if the parent view doesn\'t rerender']() {

[GLIMMER2] Moves unbound tests #13137

[GLIMMER2] Moves unbound tests #13137

Conversation

chadhietala commented Mar 20, 2016 • edited Loading

chadhietala Mar 20, 2016

Choose a reason for hiding this comment

chadhietala Mar 20, 2016

Choose a reason for hiding this comment

chadhietala Mar 20, 2016

Choose a reason for hiding this comment

chadhietala Mar 20, 2016

Choose a reason for hiding this comment

chadhietala commented Mar 20, 2016

chadhietala commented Mar 20, 2016

chancancode Mar 20, 2016

Choose a reason for hiding this comment

chancancode Mar 22, 2016

Choose a reason for hiding this comment

chancancode commented Mar 22, 2016

chancancode commented Mar 22, 2016

chancancode commented Mar 22, 2016

chadhietala commented Mar 23, 2016

homu commented Mar 23, 2016

chancancode Mar 24, 2016

Choose a reason for hiding this comment

chancancode commented Mar 24, 2016

chadhietala commented Mar 20, 2016 •

edited

Loading