ember.js: default component unit test is broken for glimmer components

OK, seriously though: The patterns that exist that let you gain access to the component instance from your rendering tests are expressive enough for these use cases without forcing component’s to be instantiated in a completely foreign and unnatural way.

For example:

module('my-thing', function(hooks) {
  setupRenderingTest(hooks);

  test('something that wants to exercise an internal method', async function(assert) {
    let component;

    this.owner.register('component:my-thing', class extends MyThing {
       constructor(owner, args) {
         super(owner, args);
         component = this;
       }
    });
    
    await render(hbs`<MyThing />`);

    // now I can interact with `component` without issue
  });
});

This pattern can easily be moved into something more easily shared if you have more than one test that uses it:

module('my-thing', function(hooks) {
  setupRenderingTest(hooks);

  hooks.beforeEach(function() {
    let testContext = this;
    this.owner.register('component:my-thing', class extends MyThing {
       constructor(owner, args) {
         super(owner, args);
         testContext.component = this;
       }
    });
  });

  test('something that wants to exercise an internal method', async function(assert) {
    await render(hbs`<MyThing />`);

    // now I can interact with `this.component` without issue
  });
});

@gabrielgrant yes that’s what I’ve done too, and there definitely are situations where this is beneficial. There are also others where it’s not. The choice of whether to use a functional approach or a stateful OOP approach should be left to the developer, or at least consistent within the framework, and not forced upon depending on whether one needs to write tests or not

Essentially, the “Ember way” prescribed in guides:

• Use @tracked!
• Use getters!
• Use args and actions on your component

BUT… you want to write tests?

• Drop all the above and use stateless functions

The situations where doing this is unwieldy are when the component has complex state due to its args and tracked properties, but then you have simultaneously force a “stateless” approach on it by having a bunch of getters call extracted functions:

get chartData() {
  return computeResult(this.args.data, this.trackedSettings);
}

It adds no value because it is actually very dependent on state, but then the code is all over the place just for the sake of tests

The solution you provided is far more reasonable. It should suit most use cases and make it easier to migrate existing unit tests.

👍👍 going to close now, as I think my suggestion in https://github.com/emberjs/ember.js/issues/18877#issuecomment-675758868 is the path forward when you absolutely must have this functionality.

One more option - using debug API - https://gist.github.com/lifeart/0484b374a3712d96f393cdd5d7c9cd49

To be clear, I think rendering tests are great. I simply meant it’s not the right tool for every job and as a work around we’re being asked to use a much more cumbersome alternative.

The solution you provided is far more reasonable. It should suit most use cases and make it easier to migrate existing unit tests. Thanks!

@gabrielgrant I did hear what you were saying earlier and I understand that, and I’m not refuting your good experience. However just because it turned out good in your case doesn’t mean it’s good in all cases. Please don’t invalidate other people’s experiences because it happened to to turn out good for your personal situation

Do all your component getters call separate, external functions? I’m assuming not – because in some cases it doesn’t make sense to. In your case it was a better refactor AND it allowed it to be tested. I’m saying there are cases where extracting it out allows it to be tested BUT is not a better refactor because of the dependency on args and states.

You changed your code to allow it to be tested, and coincidentally that was a better refactor – that’s great. However, the decision to extract code into functions should be separate from whether code can be tested or not, because in some cases this is not a better refactor:

// Typical chart component
export default class ChartComponent extends Component {
  get data() {
    const { data, options } = this.args;

    return options.showSpecial ? data.concat(this.specialDataset) : data;
  }
}

// Refactor so it can be tested
export default class ChartComponent extends Component {
  get data() {
    const { data, options } = this.args;

    return computeData(data, options, this.specialDataset);
  }
}

function computeData(data, options, specialDataset) {
  return options.showSpecial ? data.concat(specialDataset) : data;
}

Imagine having to do this for more than one getter. There is no meaningful refactor here by extracting the code. If I had multiple getters written this way, the first question someone would ask reading the code would be “why are these extracted?” and the response “just so they can be tested” would undoubtedly illicit a facepalm

I would want to just use an integration test and be done with this argument but I can’t. Would love to see how you would handle this.

@wongpeiyi agree that creating a bunch of these stub methods can be annoying in a few cases, but in general, this separation is precisely what i’ve found useful: the tests for how that stateful data gets initially computed/used in computation has unit tests on stateless functions; how the state gets established and how the results of those computations are used is tested through simple rendering tests.

In what you’re calling the “OOP” approach, testing the computation method on a component requires the test to know about, and properly set up, all the required internal state of the component. This kind of poking around internal object state from a test feels like it creates an unnecessary coupling between test and implementation. The stateless computation approach keeps those implementation details out of the tests – the knowledge of what state is used in a given method doesn’t need to leak outside of the method’s own implementation, and tests can focus on testing behavior

@schleifer-john the discussions we’ve had on the core team generally come back to “what is the ‘unit’ you are testing”. This is also why “integration” tests were renamed to “rendering” tests - the logic is that standalone component rendering tests are unit tests, because they are testing the most fundamental unit of code.

One of the common antipatterns we’ve seen, for instance, is users unit testing a component on its own without considering its interactions with any of its child components. You may say that makes sense though, because that would be an integration - but when you look at a component as a function that receives arguments and produces HTML, then that assumption kind of changes. If you were unit testing the following function:

function someUtility() {
  let value = someOtherUtility();
  return yetAnotherUtility(value);
}

You wouldn’t mock the someOtherUtility and yetAnotherUtility functions so that you’re testing someUtility on its own, because you then have a test that is testing too little - it’s effectively meaningless. The same goes for testing without considering lifecycle, which is pretty fundamental to components.

All that said, I do think there are places for testing business logic independent of rendering. There are a few options there:

1. Pull the code out into a utility function, which can be tested separately
2. If the code is stateful, create a delegate class that contains the logic and state separately and make an instance of it on the component:

   // before
   class MyComponent {
     @tracked state = 123;
   
     doSomething() {
       this.state++;
     }
   }
   
   // after
   class MyDelegate {
     @tracked state = 123;
   
     doSomething() {
       this.state++;
     }
   }
   
   class MyComponent {
     delegate = new MyDelegate();
   
     get state() {
       return this.delegate.state;
     }
   
     doSomething() {
       this.delegate.doSomething();
     }
   }
   

3. Extract the functionality to a service if it involves DI and other shared state

I’ll raise your concerns again with the core team as well to keep the conversation going, I do want to make sure your concerns are addressed and your team feels like they have a solid, well thought out path forward.

@pzuraq I know the speed of running the test case suite has gotten better now, but historically running integration tests was much slower performance-wise vs unit tests. I believe this is what originally led us down the route of favoring unit tests vs integration tests at the component level.

I also come from more of a Java / Spring background where Unit Tests are more ingrained into the regular development process, so hearing that there aren’t use cases for being able to isolate and test units of code seems like a radical deviation from what the rest of the software industry as a whole is doing.

Willing to give integration tests in Glimmer components a try and I can see where only having integration tests could work very well for simple components, but I don’t think this “one-size-fits-all” approach works for more complex components.

There are plenty of articles out there explaining the benefits of Unit Testing and I believe the same applies here for components. For more complex components in particular, we need the ability to isolate and test specific logic at the unit level without having to wire up and render the entire component. Maybe integration tests could work in these cases too, but the perception here is that it feels like the Ember framework is losing features / functionality.