From 9c07150b68c11111a3da44dd1926e4ddeca43751 Mon Sep 17 00:00:00 2001 From: Chris Freeman Date: Wed, 1 Dec 2021 11:57:29 -0700 Subject: [PATCH 01/13] first pass --- text/0000-remove-set-get-in-tests.md | 141 +++++++++++++++++++++++++++ 1 file changed, 141 insertions(+) create mode 100644 text/0000-remove-set-get-in-tests.md diff --git a/text/0000-remove-set-get-in-tests.md b/text/0000-remove-set-get-in-tests.md new file mode 100644 index 0000000000..7120864067 --- /dev/null +++ b/text/0000-remove-set-get-in-tests.md @@ -0,0 +1,141 @@ +--- +Stage: Accepted +Start Date: 2021-11-30 +Release Date: Unreleased +Release Versions: + ember-source: vX.Y.Z + ember-data: vX.Y.Z +Relevant Team(s): Ember +RFC PR: +--- + + + +# + +## Summary + +Introduce new testing utilities that remove the need for the use of `get`/`set` on `this` in test contexts. This will make rendering tests behave more like application code, and also greatly simplify the process of writing rendering tests in TypeScript since it will remove the need to append type information to `this` as properties are set. + +## Motivation + +Current rendering tests require developers to set values on `this` in order for them to actually be present in the template that gets rendered as part of the test. For example, if we're testing that a component correctly displays a value that gets passed in as an argument, we'd do something like: + +```js +this.set('name', 'foo'); + +await render(hbs``); + +assert.dom('[data-test-name]').hasText(this.get('name')); +``` + +This approach has two main issues: + +1. It's not consistent with any other programming model in Ember +1. It makes writing tests in TypeScript a huge pain + +On the first point: `this.set` is essentially irrelevant everwhere else in Ember. In a post-Octane world, we no longer need to `get` or `set`, and it is, in fact, frequently discouraged as an unnecessary complication. Furthermore, `this.set` inside of a test is even stranger than it was in application code since `set` in tests also manipulates the run loop and flushes the entirety of DOM (in additon to actually setting the value of a property on `this`), and the value of `this` is extremely murky in the first place. The fact that `this` is both the test context _and_ a sort of ersatz scope for the surrounding "template" of the rendering test is not exactly intuitive or easy to teach. + +On the second point: having to set properties on `this` leads to a whole bunch of papercuts when writing tests in TypeScript. [As discussed in the `ember-cli-typescript` docs](https://docs.ember-cli-typescript.com/ember/testing#the-testcontext), since rendering tests require developers to arbitrarily add (and change) properties on `this`, you have to define (and re-define) the interface for `this` to accomodate all of those properties just to prevent TypeScript from complaining that each of the values that were set don't exist on the `TestContext` type. + +The goal of this RFC, then, is to greatly simplify the testing model by removing the need for `get` and `set` in rendering tests, thereby reducing the reliance on `this`. + +## Detailed design + +We will introduce several new/refactored utilities for use in rendering tests: + +1. A new version of `render` that accepts a component rather than a template. +1. A new `scopedTemplate` function that accepts a template and returns a component _and is lexically scoped_ +1. A new `rerender` function that would be used after the initial `render` call + +Rendering tests currently work by rendering a template that is bound to the `this` of the surrounding test file. This is why developers have to call `this.set` in the first place, since the value of `this` in a given test is doing double duty as both the actual test context AND the backing object for the template that eventually gets rendered. Since we want to move away from this very behavior, we need a version of `render` that accepts a fully-formed component instead, since components have their own context and are self-contained. + +Once we've got this new version of `render`, we also need a convenient way of creating components to pass to it. This is where the `scopedTemplate` function comes in. `scopedTemplate` would accept a template (created using the `hbs` macro today, or the `