TestBed.configureTestingModule Performance Issue

ollwenjones commented 7 years ago

I'm submitting a ... (check one with "x")

[ ] bug report 
[x] feature request
[ ] support request

Current behavior

Importing modules into TestBed.configureTestingModule can slow tests down substantially. We have a 'shared module' with parts, that lots of our components use, including a third party library or two. It is very convenient to import this module as a rather than cherry pick in the tests, however testing a small component yesterday, I saw test bootstrap taking two whole seconds and had to cherry-pick dependencies to make tests run reliably.

Expected behavior

TestBed.configureTestingModule should be performant.

Minimal reproduction of the problem with instructions

In this plunker

go to the src/simple.spec.ts and comment and un-comment the SharedModule import into configureTestingModule.
Observe time it takes for test to run.

I am seeing a jump from 0.079s to 0.241s. (~= 3x slower). Multiply that by 5 cases and this simple test takes a whole second. Make the shared module larger and you have real problems.

What is the motivation / use case for changing the behavior?

Slow tests hurt TDD workflow
Slow enough tests disconnect karma from the browser and fail the suite.

Please tell us about your environment:

Win10, VsCode, Webpack,

Angular version: 2.0.X

Yes.
Browser: Locally, PhantomJS. Plunk tested in Chrome.
Language: TypeScript (though tests are transpiled to ES5 prior to running, naturally)

Note: Even as I write this, I wonder if it's a reasonable expectation for large modules to go through TestBed quickly? Maybe I'm just doing something wrong architecturally, (use multiple, smaller shared modules, not one big one, etc.) However, tests are time-consuming enough without having to track down every individual dependency of a more complex component just to get started.

ollwenjones commented 7 years ago

Seems like the goal is really to have a fresh component instance each time, and for the most part the testing module is going to be static for a particular .spec file. It also seems like configuring the module at each iteration is the part that is really adding on the time. A better pattern could be configuring the module once, then testModule.createComponent multiple times.

TestBed.configureTestingModule once and calling TestBed.createComponent on that module multiple times doesn't work, though... maybe I'm missing an obvious way to preserve module state between iterations?

Edit: FWIW I tried logging the timestamp and it looks like most of the time passes during TestBed.createComponent, not TestBed.configureTestingModule - however I still suspect it's related to module configuration, since adding things to imported modules adds to the time, whether or not those imports are used in the component being tested.

vmandy commented 7 years ago

I have a noticed unit test execution performance issues that seem directly related to this issue.

I have an ionic 2 / angular 2 app. When trying to unit test my components using TestBed and ComponentFixture classes, there is easily 1-2 seconds spent in the setup of each test that seems directly related to the imports: [IonicModule.forRoot(MyComponent)] that I've added to my testing module via the TestBed.configureTestingModule({...}) method.

When I remove the import, my tests run much faster, but they obviously fail because my component's template no longer compiles.

ollwenjones commented 7 years ago

@vmandy I've been working around this by doing one or more of:

spending more time curating just what I need in the test module
mocking the component dependencies that are time-consuming to bootstrap
overriding the template of the component I'm testing so it doesn't depend on much.

All of which are time consuming and sub-optimal 👎 (except from the point of view that I am really forced to isolate the unit I am testing?)

It seems like there are some theoretical optimizations to this at least, because I know these components are not taking 1-2 secs to bootstrap at run-time.

FlemmingBehrend commented 7 years ago

Iam having similar problems. Thought it was caused by wallaby, but it seems to be the TestBed.

https://github.com/wallabyjs/public/issues/885

mlakmal commented 7 years ago

I am seeing the same issue. Any unit test with component compilation is taking close to 1sec...

ollwenjones commented 7 years ago

Unfortunately I was pulled off the Angular 2 project and haven't looked at this in quite some time. It seems like a lot of the complications of TestBed (test-module setup, generally, not just performance) could be avoided by an approach similar to the enzyme.shallow render approach in the React ecosystem. - where just one unit is rendered and none of it's dependencies - the same thing can be accomplished by over-riding an Angular component's template in the .spec, but that is often tedious/time-consuming as well.

mlakmal commented 7 years ago

we have close to 2000 test cases getting executed just under 1 min in Angular 1 where Angular 2 takes 1min to execute just 100 test cases. It would be nice if we can get a resolution for this issue as it hinders the testing capabilities in large applications.

ollwenjones commented 7 years ago

@mlakmal I think there are lots of folks relying heavily on e2e tests, or just testing functional logic in component classes

let component = new ComponentClass(mockDep1, mockDep2);
expect(component.doAddition(2,2)).toBe(4);

Which doesn't test any of the template rendering (template logic)... but it does allow easy testing of class logic at least

mlakmal commented 7 years ago

@ollwenjones thanks, i am going to try that out. i really doesn't want to test any template logic since most of that gets covered by our automated tests, so just testing the component class code should be enough.

ollwenjones commented 7 years ago

@mlakmal sweet! Makes me happy to contribute something that actually helps someone. 🍻 - FWIW I'm often torn about template-rendering unit tests, because it's so easy to slip into testing the framework.

michaelbromley commented 7 years ago

I'm in the process of upgrading a pretty big project from rc.4 to 2.3, so it's the first time we're using the NgModule approach.

We have ~1000 unit tests, with maybe 1/2 of those testing components. Prior to upgrade, tests run pretty quickly - around 30s.

After upgrade, they take at leat twice as long, but the worst thing is that they seem kind of "flaky" - Karma will periodically lost the connection to Chrome and often I will need to refresh Chrome multiple times. I suspect that something is timing out but haven't isolated it yet.

@ollwenjones regarding "shallow" component tests, did you know about the NO_ERRORS_SCHEMA that you can set in the testing module?

import { NO_ERRORS_SCHEMA} from '@angular/core';
// ...
TestBed.configureTestingModule({
  declarations: [ /*... whatever */ ],
  schemas: [NO_ERRORS_SCHEMA]
});

This means that the compiler just ignores any elements it does not recognize, meaning you do not need to declare all the components used in the template of the component under test.

marclaval commented 7 years ago

Found the same issue in Angular's tests: #13500 I was able to improve the situation by creating bespoke testing modules for each test.

ollwenjones commented 7 years ago

@michaelbromley 😮 I had no idea about NO_ERRORS_SCHEMA would have / will probably save me hours - also an opportunity to hopefully speed up some of these tests, as the leaner the test-module the faster TestBed seems to go.

FlemmingBehrend commented 7 years ago

@michaelbromley that also helped me out a lot. Removing MaterialModule.forRoot() from my imports really speed up my tests. 👍

awerlang commented 7 years ago

Ideally, we should be able to call TestBed.configureTestingModule() inside a beforeAll(). It just doesn't work because of this line. This is reseting the modules before each test. Perhaps it shouldn't matter, but it does because Angular spends quite a lot of time compiling components in JiT mode. Reseting it throws away this information. Recording a profiling session on karma window led me to this conclusion.

IMO, this should be done:

Be able to configure a module in a beforeAll();
Cache compiled components

Another way to fix that is making reset testing module explicit, but this may break user code.

gonzofish commented 7 years ago

Is it clear that this isn't something to do with karma-phantom? The tests fly in Chrome for me, but Phantom takes at least a second per test. I've mocked the dependencies out as much as possible and they still run slow in Phantom.

Necroskillz commented 7 years ago

phantom is very slow for me (using karma-webpack, but this is a different issue. I use Chrome now, and you can visually see as the tests run when there are tests that use a lot of components that have to be compiled it gets stuck for a second and runs slowly until it hits service tests which run fast.

Configuring module in beforeAll is not so convenient, because i want to create new mocks after each run and have them (and other providers) in a fresh state.

Caching the compiled components is the way to go imo.

philipooo commented 7 years ago

You can also use electron and karma-electron to run your tests headless. Its nearly as fast as chrome. Phantomjs is about 8 times slower for me.

Cheers

Necros notifications@github.com schrieb am Mi., 8. März 2017, 23:49:

phantom is very slow for me (using karma-webpack, but this is a different issue. I use Chrome now, and you can visually see as the tests run when there are tests that use a lot of components that have to be compiled it gets stuck for a second and runs slowly until it hits service tests which run fast.

Configuring module in beforeAll is not so convenient, because i want to create new mocks after each run and have them (and other providers) in a fresh state.

Caching the compiled components is the way to go imo.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/angular/angular/issues/12409#issuecomment-285195615, or mute the thread https://github.com/notifications/unsubscribe-auth/ABn5_18VjaBSPZQX1ZQ45mveN2PFMoNBks5rjzB-gaJpZM4KcS5I .

michalmo commented 7 years ago

AOT support would help here too.

thymikee commented 7 years ago

@philipooo with slight modifications to some of Jasmine references (jasmine.createSpyObj() specifically), you can also run your tests without any browser – by using Jest instead of Karma.

For as few as 35 files with total 100 tests Jest has been able to speedup my tests 2.5x (vs Karma on Chrome). Tests are now running in parallel, completely isolated, not to mention superior watch mode with instant feedback.

I've written about it here, if anybody's curious: https://www.xfive.co/blog/testing-angular-faster-jest/.

There's also an issue to integrate it directly into CLI (e.g. under a flag: ng test --jest): https://github.com/angular/angular-cli/issues/4543.

ollwenjones commented 7 years ago

@thymikee I'm super excited to hear that. I've had muuuch better experience testing React code with Jest than I ever have with Angular, and I've been persuaded that end-users of trustworthy frameworks don't need to do as much browser-testing.

Last time I checked Jest was using jsdom which did not play nice with zone.js is this no longer the case? 🤞 I'll have to read your blog.

thymikee commented 7 years ago

So jsdom is not the problem. It's just that tests must run in Zone.js context, which is done by this little lib: jest-zone-patch

jsgoupil commented 7 years ago

Testing with HTML components is just a pain... I have about 500 unit tests that are 90% looking at the DOM, it takes me 40 minutes to run them all... at this rate, Chrome crashes when it reaches a RAM level too high... the result is that I have to batch my tests by 50. Painful as hell...

There are definitely some memory leaks here and there...

yairgo commented 7 years ago

bueller?

oferh commented 7 years ago

Found out that using FormsModule considerably slows down compileComponents mainly using ngModel.

For example here is a plunker which has a component with 10 inputs and it takes about 3 times more when using ngModel.

dblVs commented 7 years ago

@juliemr Hey, I wanted to do a feature request to add a schema that does shallow rendering. Shall keep the stuff here (since what @ollwenjones wrote pretty much is what i want) or create a new issue? Basically it's:

Being able to instantiate the child components, BUT not it's template. (this is a bit more complex than it sound)
Still having errors when bound to wrong property on the component (unlike error schema)

At the moment I'm creating stub components in my project to create blank components and have them implement same interface to make them consistent the inputs and outputs and makes it easier to my users to test the template of their component. It becomes really difficult once you include stuff like ContentChildren logic combined with templateOutlet directive usage or transclusion.

Possible solution is use normal schema ot 1st component then for the child components use the NO_ERROR_SCHEMA.

ollwenjones commented 7 years ago

So we recently switched to Jest from webpack > karma/phantom/chrome, using the great instructions / tools @thymikee 🎊 which speeds things up a bit (the pre-test webpack build step is removed at least) and simplified setup tremendously - but overall Angular testing still feels like a struggle.

The main reason is configuring testing modules is still a pain - results with NO_ERRORS_SCHEMA are spotty, and certain things that are destined to slow things down (like things from @angular/material and DynamicFormsModule) have to be included (so this one simple test I'm working on still takes 6 seconds.) Thanks @oferh for the plunk definitively demonstrating this.

Leads me to say that @dblVs, I think you should do a feature request. I think a positive suggestion with some good design ideas will probably be better received by core people than my negative, "this is slow... 😢"

My opinion is that without such a tool, we aren't empowered by the current tooling to write unit tests at all. All these tests with deep component rendering and all dependencies down the tree provided in the test-module for injection, etc. are integration tests, and integration tests are harder to write.

mlakmal commented 6 years ago

if it helps anyone, I tried Electron headless browser with angular 2 unit tests and it performs far better than PhantomJS.

mlc-mlapis commented 6 years ago

Yes, PhantomJS is really very old. But the latest version of Chrome can also run as a headless browser and it will be the fastest one.

ChenReuven commented 6 years ago

@ollwenjones i totally agree, this is an very important/critical feature request. if we want to develop via TDD(also normal testing phase) its very hard and we are struggle with the test runner time performance

vvasabi commented 6 years ago

This is my current work around:

const oldResetTestingModule = TestBed.resetTestingModule;
beforeAll(done => (async () => {
  TestBed.resetTestingModule();
  TestBed.configureTestingModule({
    // ...
  });
  await TestBed.compileComponents();

  // prevent Angular from resetting testing module
  TestBed.resetTestingModule = () => TestBed;
})().then(done).catch(done.fail));

afterAll(() => {
  // reinstate resetTestingModule method
  TestBed.resetTestingModule = oldResetTestingModule;
  TestBed.resetTestingModule();
});

This way, it only needs to run configureTestingModule and compileComponents once per spec file. After the testing module is initialised, running TestBed.createComponent() is fairly fast.

rbinsztock commented 6 years ago

@vvasabi interesting workaround, one spec test file was very slow, because inside a beforeEach we defined the module who includes a module with many components and some mocked services (with providers array and useClass) Sadly some tests are failing after using your fix. We also use AOT who maybe slowdown the compile.

We also use overrideComponent inside beforeEach as the service is only used in this component.

fixture = TestBed.overrideComponent(ListingFormComponent, {
      set: {
        providers: [
          { provide: DisplayFieldService, useClass: MockDisplayFieldService }
        ]
      }
    }).createComponent(ListingFormComponent);

But the main problem is clearly the include of a "big" module with many components inside the configureTestingModule.

Time after fix : Executed 53 of 148 (3 FAILED) ERROR (1.261 secs / 0.327 secs) running smooth but 3 failing tests now maybe more as the test suite stopped. Time before Fix : Chrome 59.0.3071 (Mac OS X 10.12.5): Executed 63 of 148 (skipped 85) SUCCESS (44.235 secs / 44.164 secs) with some freeze execution during tests.

Still investigating.

vvasabi commented 6 years ago

@rbinsztock I would try to isolate the failing tests into their own spec files without my hack applied. Because my hack prevents Angular from resetting the test bed after each test is run, some tests that change states in shared services (i.e. pollute the test bed) may cause other tests to break. For example, if test 1 makes some changes to your MockDisplayFieldService, then test 2 will get back the same MockDisplayFieldService instance with the changes still applied.

Once you figure out which tests are changing the states of shared services and causing others to break, you can try to undo the changes after the assertions. Then, move back those tests that were isolated, and see if they can now pass. Hope this helps.

Edit: I just noticed that your MockDisplayFieldService is provided by ListingFormComponent, so while it likely has a new instance created for each test, other shared services might not.

giniedp commented 6 years ago

wow great workaround. Tested it in one spec file.

Before

Finished in 6.282 secs / 7.383 secs @ 16:56:55 GMT+0200 (CEST)

After

Finished in 0.17 secs / 0.202 secs @ 16:59:56 GMT+0200 (CEST)

But since it is still a workaround i can not let my dev group exploit that wildly. We have 1000+ unit tests. The whole suite takes about 10 minutes to run. Would love to see an official solution soon.

ollwenjones commented 6 years ago

I started wondering if this isn't just too much to ask, because of how much compilation has to happen just to have one Angular thing (typescript, decorator annotations, template parsing, all the ng-modules for DI, aot in some cases, etc.) - maybe it's just not be reasonable to expect them to run like some pure JS or even pure TS test? 🙁

giniedp commented 6 years ago

I think it is ok for a testmodule to be slow on construction time. But we should have the choice whether to create the TestBed before all or before each test case. Right now we are forced to the latter and thus the only option to speedup the tests is to clutter multiple expectations into one single test unit. 👎

Shireilia commented 6 years ago

Complelty agree with @giniedp here.

In our case we have two seperate repositories, one with 2500+ test and the other that will reach the 1000 before the end of the month.

The first one still imports many modules in the TestBed configuration, leading to 6-7 minutes to complete a full run. I've started to remove them one by one as i work on each feature to improve the speed. The second one, created way after that, i've said to my team to use as many mocks as possible and to avoid at all cost importing anything in the testbed, and in general, group multiple expect in a single test (wich IMO is wrong). This is muuuuch faster for a full test run (without compilation, about 20 seconds), but the compilation time still take at least a minute if not up to 2 or 3, wich is clearly annoying.

I'd love to see a big performance improvement on tests, since it's where i lose 50% of my time when developping a feature.

thymikee commented 6 years ago

@Shireilia try Jest (with jest-preset-angular) so you can skip the compilation step, parallelise tests and have smarter watch mode

Shireilia commented 6 years ago

@thymikee Thanks, i'll give it a try in the coming week :)

vvasabi commented 6 years ago

@thymikee Thanks for your work on the preset. Jest works really well for my project. A few observations:

Jest runs faster than Karma because it bypasses Webpack and has the ability to run multiple spec files concurrently. However, the tests, that require Angular components to be instantiated, actually take longer time to run than Karma in Chrome. I am guessing that the problem might be jsdom not as efficient as Blink. Though, some further investigations will be required.
Since Jest does not require all modules to be loaded in order for tests to run, the fewer modules that a test has to import, the faster it will finish. I spent some time isolating modules that did not have to be imported (directly or indirectly), and it gave me some mild improvement on the lap times. (For example, some modules, that weren’t used, got exported by an index.ts file and ended up in the import graph.)
My workaround above is applicable when using Jest, as well.
I noticed that spec files that did not have a unique name would break IntelliJ’s ability to set breakpoints in them. I will try to create a test repo and open an issue in your repo. (Edit: issue opened)

Thanks again. Jest is awesome.

DrewLandgrave commented 6 years ago

@vvasabi thanks for the workaround it drastically sped up my tests. One question though, would there be a way to have a central module that the spec files just use. E.g. go ahead and include everything in one module then have all the spec files use it?

May be an anti pattern but the slowdown for me now is it takes time to set the module of for each spec file. I have 20 so far and they are growing (one for each component, service, directive, etc) So the slow down comes when each spec file is hit. Once the module is built the subsequent it statments run in no time.

Thanks again for the workaround

vvasabi commented 6 years ago

@Rhonun Glad to hear that my hack works for you. You can certainly build a common testing module. Just export the module metadata from one file, and your spec files can just import and reuse.

If you want to go one step further and make sure my hack is not repeated in every spec file, consider setting up a util function like this:

const resetTestingModule = TestBed.resetTestingModule,
  preventAngularFromResetting = () => TestBed.resetTestingModule = () => TestBed;
  allowAngularToReset = () => TestBed.resetTestingModule = resetTestingModule;
export const setUpTestBed = (moduleDef: TestModuleMetadata) => {
  beforeAll(async(async () => {
    resetTestingModule();
    preventAngularFromResetting();
    TestBed.configureTestingModule(moduleDef);
    await TestBed.compileComponents();
  }));

  afterAll(() => resetTestBed());
};

Then, just call it inside the outermost describe block of your spec files. Hope this helps.

dasAnderl commented 6 years ago

@vvasabi ur work around indeed works well. i cut down around 250 tests from 45 secs to about 15 secs. i really think the ng team should provide the option to setup the testing module in beforeAll as this saves a lot of time, especially in specs with a lot of its.

only drawback so far is that in some specs the dom interaction seems to be broken with the new approach (e.g. setting input values, clicking btns )

another drawback is that it doesnt seem possible to use beforeAll with @vvasabi s approach in some specs and beforeEach in other specs in the same suite.

vvasabi commented 6 years ago

@dasAnderl Glad to hear that my trick worked for you.

some specs the dom interaction seems to be broken (e.g. setting input values, clicking btns)

Since the testing module is now reused across multiple tests, any test that mutates a shared service can potentially affect the result of other tests. One quick way to confirm this is to run the broken tests individually. If they pass when run alone, then they fail because of polluted TestBed.

drawback is that it doesnt seem possible to use beforeAll in some specs and beforeEach in other specs in the same suite

Yes, the workaround will force you to group up tests that can reuse the same TestBed configuration in one describe block. Ones that cannot will have to live in their own compartments. I would like to argue that this might be a cleaner code organisation anyways, whether or not the hack is applied.

BurningDog commented 6 years ago

@vvasabi THANK YOU!!! This has been really helpful in getting my test run times down. I'm probably adding far too much metadata to each test which means I need to refactor my components to be more modular (which I'm not quite sure how to do), but in the meantime your suggestion has led to a massive gain in test execution performance.

Before: Executed 502 of 502 (4 FAILED) (16 mins 24.944 secs / 15 mins 51.429 secs)

After: Executed 502 of 502 SUCCESS (2 mins 53.59 secs / 2 mins 5.877 secs)

Still slower than I'd like, but biiiiig improvement!

(Yes, these tests run on a particularly slow environment, where we often get async timeouts that don't happen locally. Simply running the tests faster seems to solve these).

In case someone else needs more specific details for how to apply your code, here it is.

Create a new file called src/test.common.spec.ts (FYI: I had to make some small changes here compared to what you had above). The .spec.ts extension is so that the file isn't included in builds - otherwise ng build will complain with a Cannot find name 'beforeAll' error.

Add the following into the file:

import { TestBed, async, TestModuleMetadata } from '@angular/core/testing';

const resetTestingModule = TestBed.resetTestingModule,
  preventAngularFromResetting = () => TestBed.resetTestingModule = () => TestBed;
let allowAngularToReset = () => TestBed.resetTestingModule = resetTestingModule;

export const setUpTestBed = (moduleDef: TestModuleMetadata) => {
  beforeAll(done => (async () => {
    resetTestingModule();
    preventAngularFromResetting();
    TestBed.configureTestingModule(moduleDef);
    await TestBed.compileComponents();

    // prevent Angular from resetting testing module
    TestBed.resetTestingModule = () => TestBed;
  })().then(done).catch(done.fail));

  afterAll(() => allowAngularToReset());
};

In a specific .component.spec.ts file there may be something like the following:

describe('SomeComponent', () => {
  beforeEach(async(() => {
    TestBed.configureTestingModule({
      declarations: [ OneComponent, TwoComponent ],
      imports: [ SharedModule ],
      providers: [ HttpService ]
    }).compileComponents();
  }));

This then becomes:

describe('SomeComponent', () => {
  let moduleDef: TestModuleMetadata = {
    declarations: [ OneComponent, TwoComponent ],
    imports: [ SharedModule ],
    providers: [ HttpService ]
  };
  setUpTestBed(moduleDef);

Run the tests with ng test

It's kinda obvious, but don't use this pattern any time you need the component/service/whatever you're testing state to be reset between tests.

willgm commented 6 years ago

I tested @vvasabi`s work around, and it really worked! My test suit is now almost 10x faster!

Is there any official response from the core team on this subject? Is there any plans to make it faster without this kind of work around?

EDIT: I just found the PR #17710 that would solve this issue, but it is more than 2 months without answer.

brian428 commented 6 years ago

I also have a similar issue in the tracker (https://github.com/angular/angular/issues/13963) regarding the fact that all test dependencies are recompiled for every test method. I think if the TestBed would just compile once and then re-use the compiled output for subsequent test methods, that would speed everything up a lot.

ronnyek commented 6 years ago

I just used this mechanism... I had 100 tests that took about 2min48sec. Converted them over, and the execute in 8sec

willgm commented 6 years ago

@robwormald and @alxhub, may we have an update on this tread? There is also an open PR to fix that without any feedback for a long time.

One of the main features of Angular is testability, and this performance issue is really a problem. I have a production app with ~1500, and it is impossible to use TDD without this ugly workaround.

dasAnderl commented 6 years ago

totally agree. i also use the workaround, i encapsulated it so it is well usable. however this should be sth ng offers built in (setup in beforeAll). also in general the test setup fells clumsy in ng. this includes the setup....and injection of services as well....have no comparison how it is in e.g. react...i have the feeling it could be easier somehow...i encapsulated for better usability like this:

let comp: Comp; setUpTestBed(CfModule, () => comp = new Comp(CfComponent));

for injection i still use the clumsy stuff:

let _restService: RestService; let _dataService: DataService;

beforeEach(inject([RestService, DataService], (restService: RestService, dataService: DataService) => { _restService = restService; _dataService = dataService; }));

angular / angular

TestBed.configureTestingModule Performance Issue #12409