Regression with v0.5.26

[workmanw]

We were alerted to some test failures this week with ember-beta (2.7.0-beta.1). After diving in I discovered the cause was actually due to a change with ember-test-helpers.

Prior to this latest release, if you called this.render more than once in a single test it would append the rendered component. Now if you call this.render more than once it replaces that component.

So you could argue that calling this.render multiple times in a single test is silly and we should restructure our tests. I can happily explain my use case if anyone is interested. But regardless this caused some test failures for us, I would expect it to do the same for others.

The regression was introduced with: 696da08 . CC: @rwjblue

[rwjblue]

I am definitely interested in the specific use case for having each subsequent this.render doing an append (instead of replace). This was never intended behavior, and actually had many bugs (the test harness from each this.render other than the last one was never cleaned up or destroyed).

I'm not 100% sure how to replicate that older behavior, support Ember 2.7.0-beta.1, and avoid leaking...

[rwjblue]

Prior to this latest release, if you called this.render more than once in a single test it would append the rendered component. Now if you call this.render more than once it replaces that component.

How do you observe this appending? I believe that calling this.$() in the test harness would only return the DOM from the last this.render() call (see here and here).

So you could argue that calling this.render multiple times in a single test is silly and we should restructure our tests.

FWIW, I do not think it is silly. We added the this.clearRender() API specifically to support calling this.render() twice.

[workmanw]

@rwjblue Absolutely!

For some of our integration tests we have boilerplate components that must be rendered to test the end-to-end aspects of the integration. We render these boilerplate components in the beforeEach so we don't have to render them in each test.

An example of that is our upload manager. We have a components/upload-button that let's users uploading things to the server. While the upload is processing, the service/upload-manager keeps track of all pending uploads. We also have a components/upload-status. So our test looks something like this:

moduleForComponent('service:modal', name, {
  integration: true,
  beforeEach() {
    this.render(hbs`{{upload-status}}`);
  }
}

test('Test Upload-Button', function(assert) {
  this.render(hbs`{{upload-button}}`);

  /* ... MOCK UPLOAD ... */
  /* TEST {{upload-status}} state */
});

So instead of rendering {{upload-status}} in every test, we render it once.

We had originally tried to implement a function helper that would basically concat the two templates and call this.render once, however that didn't work because the hbs helper is a preprocessor.

This is also just a one line trivial example. Doing this.render(hbs{{upload-status}} {{upload-button}}); is not really much boilerplate. But we have many other tests that have 4-6 lines of "render setup" and 10-20 tests which rely on that.

[rwjblue]

Gotcha. Thank you for explaining. I agree that this is a useful scenario to solve. I think I might do something like:

moduleForComponent('service:modal', name, {
  integration: true,
  beforeEach() {
    this.header = `{{upload-status}}`;
  }
}

test('Test Upload-Button', function(assert) {
  this.render(hbs(`${this.header}{{upload-button}}`));

  /* ... MOCK UPLOAD ... */
  /* TEST {{upload-status}} state */
});

Or possibly:

moduleForComponent('service:modal', name, {
  integration: true,
  beforeEach() {
    this.renderWrappedTemplate = (template) => {
      this.render(hbs(`{{upload-status}}${template}`));
    };
  }
}

test('Test Upload-Button', function(assert) {
  this.renderWrappedTemplate(`{{upload-button}}`);

  /* ... MOCK UPLOAD ... */
  /* TEST {{upload-status}} state */
});

---

But of course that avoids the issue of this being a breaking change between 0.5.24 and 0.5.25/26, which is both annoying and unfortunate...

[rwjblue]

Hmmm. I am not certain that those snippets work work actually. Since the hbs is a precompiler, and doesn't have access to the runtime information...

[workmanw]

@rwjblue Yeap, they will not work. We tried that too. It must be evaluated at preprocessor time.

[workmanw]

Oh yuck. I had it in my head this was related to ember-beta. But now it seems to be happening for me even with Ember 2.5 (our current version).

I think I'll just add ember-test-helpers: 0.5.24 to my package.json as a workaround for now.

[rwjblue]

Yeah, we use the same underlying implementation for 1.13 and above (we use the same implementation as 0.5.24 for < 1.13).

[rwjblue]

I'm struggling on coming up with a way forward here.

The prior system (used in ember-test-helpers@0.5.24 and prior) which simply appended DOM into the #qunit-fixture supported this, but had a number of issues:

• It would only cleanup things rendered in the last this.render invocation.
• It relied on some pretty whacky private API's (context switching on Ember.Component) that has given us much grief on various Ember versions (specifically 1.10, 1.13, and now the glimmer rendering engine refactor).

In ember-test-helpers@0.5.25+ the underlying implementation that we are using now is to treat the rendered templates like a normal route template, and use the outlet rendering system to render things. The solves both of the issues listed above, but unfortunately makes it so that you cannot combine this.render invocations.

In theory, I could add support for something similar by allowing {{outlet}} usage in each template that is this.rendered. It should be straightforward to build up the required outlet state for that, but I am unsure how odd it is to folks not intune with the inner workings to use {{outlet}} in a component integration test.

Definitely open to suggestions here...

[rwjblue]

@workmanw - One possible solution for your scenario might be:

moduleForComponent('service:modal', name, {
  integration: true,
  beforeEach() {
    this.register('template:components/x-header', hbs`{{upload-status}}`);
  }
}

test('Test Upload-Button', function(assert) {
  this.render(hbs`{{x-header}}{{upload-button}}`);

  /* ... MOCK UPLOAD ... */
  /* TEST {{upload-status}} state */
});

That still allows for things to be a bit more DRY, and avoids the need for runtime concatenation.

Thoughts?

[workmanw]

@rwjblue Yea I did read through the commits and I think I understand what you're saying. I'm in complete agreement that appending the way it use to was wrong. And having the test suit try to act like an old Ember.ContainerView would also be wrong / undesirable because it doesn't match the actual app runtime.

After reflecting on this, I really feel like the real problem here is not that I can't call this.render multiple times, but rather I can't concatenate or join template strings some how . If I could do something like this it would be ideal:

moduleForComponent('service:modal', name, {
  integration: true,
  beforeEach() {
    this.register('template:components/x-header', hbs`{{upload-status}}`);
    this.__orig__render = this.render;
    this.render = function(str) {
      this.__orig__render(hbs`{{x-header}}${str}`);
    }
  },
  afterEach() {
    this.render = this.__orig__render;
    this.__orig__render = null;
  }
}

test('Test Upload-Button', function(assert) {
  this.render(hbs`{{x-header}}{{upload-button}}`);

  /* ... MOCK UPLOAD ... */
  /* TEST {{upload-status}} state */
});

I know that probably looks hacky ... but for some of our tests, we implement our own test modules that extend moduleForComponent. A solution like this would easily be contained in a single helper and I could live with that.

Just talking it out here ... using hbs generates a precompiled object. Oh. Oh. Alright, this is going to sound completely crazy, what if I used {{partial}}. What about this:

this.render = function(template) {
  this.register('template:x-body', template);
  this.__render(hbs`{{x-header}} {{partial "x-body"}}`);
}

I wonder if the registry / container would cache that template value and thus make it impossible to call render subsequent times. Hmmm.

---

@rwjblue At this point, I'm okay with closing this issue if you are. I very much understand every piece of functionality cannot be completely maintained. I would not at all be upset if this was deemed as one of those things that's just low ROI and won't fix.

[workmanw]

@rwjblue et al -- Circling back around on some of my open issues and ran back into this guy.

My solution was actually to use a partial in just the way I described above. Overriding the render call and injecting injecting the argument value into the registry as a template for a partial. It works for me so I'm going to close this.

[rwjblue]

Thanks @workmanw