Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance Improvements (similiar ng-bullet) #449

Open
r-kernberger opened this issue Aug 21, 2020 · 21 comments
Open

Performance Improvements (similiar ng-bullet) #449

r-kernberger opened this issue Aug 21, 2020 · 21 comments

Comments

@r-kernberger
Copy link

r-kernberger commented Aug 21, 2020

Performance Improvements (similiar ng-bullet)

Feature Request

A large Unit Test (23 Tests) is running for ~10minutes.
There is a library (ng-bullet), which avoids recompiling everything
used to configure the TestBed for every test in a suite, thus increasing the overall test execution time alot (~30 seconds).
https://github.com/topnotch48/ng-bullet-workspace/blob/master/projects/ng-bullet/README.md

Downside on this library is, that it is no longer maintained.
Are there any plans, integrating the approach of ng-bullet
in jest in the near future and thus increasing the performance of jest?

To Reproduce

  • Writing a unit test with > 20 Tests.
  • Execute the test and measure execution time
  • Install ng-bullet
  • Execute the same test and measure execution time again
  • Compare Execution times

Expected behavior

Execution time of Jest unit tests will be similiar to execution time when using ng-bullet

Link to repl or repo (highly encouraged)

envinfo

System:
OS: Windows 10 10.0.17763
CPU: (12) x64 Intel(R) Core(TM) i7-8750H CPU @ 2.20GHz
Binaries:
Node: 10.15.0 - C:\Program Files\nodejs\node.EXE
npm: 6.4.1 - C:\Program Files\nodejs\npm.CMD

@r-kernberger r-kernberger added Bug Report Needs Repo Need a minimium repository to reproduce the problem Needs Triage labels Aug 21, 2020
@ahnpnl ahnpnl added Enhancement ✨ and removed Bug Report Needs Repo Need a minimium repository to reproduce the problem Needs Triage labels Aug 21, 2020
@ahnpnl
Copy link
Collaborator

ahnpnl commented Aug 21, 2020

definitely, I think it looks very promising.

@ahnpnl ahnpnl added Help Wanted 🚀 Feature Request new suggested feature labels Aug 21, 2020
@wtho
Copy link
Collaborator

wtho commented Aug 21, 2020

In the root README of the linked repository the owner recommends another project, maybe we should consider both.
Referenced project.

@splincode
Copy link

@ahnpnl
Copy link
Collaborator

ahnpnl commented Aug 21, 2020

Look at spectator, it seems to be out of the scope of the preset. Not sure how to integrate into here.

@r-kernberger
Copy link
Author

r-kernberger commented Aug 25, 2020

Not mantained
https://github.com/topnotch48/ng-bullet-workspace#update

@splincode
That was the whole point of this feature request. ng-bullet is not maintained, so it would be great if "jest-preset-angular" would integrate the performance boost of ng-bullet.

@wtho
Copy link
Collaborator

wtho commented Aug 26, 2020

@r-kernberger I see.
We currently have other features in focus, but we are always open to PRs.

I think key for success of this integration is that the interface of the tests does not change (much), so that developers have a low threshold to adopt the performance boost.

E. g. this could be an integration I would love:

  import {
    async,
    ComponentFixture,
-   TestBed
  } from '@angular/core/testing';
+ import { TestBed } from 'jest-preset-angular/speedup-testbed';

  describe('suite', () => {
    let comp, userService;
    beforeEach(() => {
      TestBed.configureTestingModule({
        providers: [ WelcomeComponent, { provide: UserService, useClass: MockUserService } ]
      });
      comp = TestBed.inject(WelcomeComponent);
      userService = TestBed.inject(UserService);
    });

    it('should not have welcome message after construction', () => {
      expect(comp.welcome).toBeUndefined();
    });
  });

@ahnpnl
Copy link
Collaborator

ahnpnl commented Aug 26, 2020

I gave a try with @ngneat/spectator, it is quite good in term of testing util convenience, not sure if it boosts or not.

@intellix
Copy link
Contributor

Is this idea still valid or is it irrelevant with the latest changes from Angular?

@ahnpnl
Copy link
Collaborator

ahnpnl commented Jan 21, 2021

Not quite sure still valid, I haven’t looked into what ng-bullet does. Ng-bullet is a test util library rather than a Jest transformer. It is more related to how tests in Angular are setup or tear down.

@rfprod
Copy link
Contributor

rfprod commented Dec 4, 2021

@ahnpnl I know the ng-bullet approach, and it is not good.
The idea is to prevent TestBed cleanup and recompilation.
It can eventually lead to sporadic issues with unit tests, and unexpected behavior.
ng-bullet introduces a hacky way to speed up unit tests that are either poorly written, or a hacky way to speed up test when computations caching and granularity should be used.

If you take a closer look at what ng-bullet does...
Check the source code with explanation below

export const configureTestSuite = (configureAction?: () => void) => {
    const testBedApi: any = getTestBed();
    const originReset = TestBed.resetTestingModule; // it backups the original resetTestingModule method that cleans up the TestBed

    beforeAll(() => {
        TestBed.resetTestingModule();
        TestBed.resetTestingModule = () => TestBed; // then in the beforeAll hook it overrides the original method by just returning the dirty TestBed
    });

    if (configureAction) {
        beforeAll((done: DoneFn) => (async () => {
            configureAction();
            await TestBed.compileComponents();
        })().then(done).catch(done.fail));
    }

    // in the afterEach hook the TestBed is cleaned up, kind of but not really
    // it just destroys fixtures, sets a flag, resets the test module ref, but everything else remains dirty
    afterEach(() => {
        testBedApi._activeFixtures.forEach((fixture: ComponentFixture<any>) => fixture.destroy());
        // reset ViewEngine TestBed
        testBedApi._instantiated = false;
        // reset Ivy TestBed
        testBedApi._testModuleRef = null;
    });

    // then in the afterEach hook it restores the original resetTestingModule method, and actually resets the TestBed
    afterAll(() => {
        TestBed.resetTestingModule = originReset;
        TestBed.resetTestingModule();
    });
};

If you asked me, I would not recommend using this approach.
I think ng-bullet is not maintained any more for reasons.
I used to use this hack quite long ago before switching to monorepos and nx dev tools, but without ng-bullet.
I would recommend using nx dev tools instead of trying to hack into the original TestBed flow. Nx dev tools has computations caching, and facilitates source code granularity.

And yes, you're right. It has nothing to do with Jest transformers.

@Xaz16
Copy link

Xaz16 commented Dec 20, 2021

Hi folks
Did you check such thing like https://github.com/swc-project/jest?
In my case it is incredible (left side swc/jest, right side ts-jest without cache due to kulshekhar/ts-jest#2696)
image
And even if my project is crap, situation on nx-examples https://github.com/nrwl/nx-examples as well incredible

I have such result:
Out of the box -- node_modules/.bin/nx run-many --all --target=test 153.06s user 8.40s system 595% cpu 27.101 total
With swc/jest -- node_modules/.bin/nx run-many --all --target=test 9.00s user 1.08s system 169% cpu 5.932 total

@ahnpnl
Copy link
Collaborator

ahnpnl commented Dec 20, 2021

Swc doesn’t support AST transformers which Angular needs

@wtho
Copy link
Collaborator

wtho commented Dec 20, 2021

@ahnpnl Isn't swc's plugin system kind of an AST transformer interface?
https://swc.rs/docs/usage/plugins
Would still require quite some work to connect the current transformers though.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Dec 20, 2021

Angular transformers require the context of Program, for example downlevel ctor requires that which swc cannot do. As long as Angular transformers require Program, it's not possible for any tools without Program context.

@splincode
Copy link

@alan-agius4 maybe angular team can help with current discussion?

@ahnpnl
Copy link
Collaborator

ahnpnl commented Dec 20, 2021

We highly depend on this angular/angular#43165 If that is implemented which the Program context is no longer required.

@alan-agius4
Copy link

The program is needed because in TypeScript you cannot get the typechecker otherwise.

This is related to this topic angular/angular#43131

@Xaz16
Copy link

Xaz16 commented Dec 20, 2021

The program is needed because in TypeScript you cannot get the typechecker otherwise.

This is related to this topic angular/angular#43131

@ahnpnl
But as I know swc/jest is not performing type cheking and even do not need it (sorry if I totaly wrong, but I'm not so familiar with it). I think big percent of devs will trade x20 faster test runs instead of typecheking in theirs specs

@alan-agius4
Copy link

The typechecker in the mentioned case is required to get certain information of symbols (which is not possible without it), this information is needed to perform the constructor transformations.

@Xaz16
Copy link

Xaz16 commented Dec 21, 2021

@ahnpnl @alan-agius4
I asked from swc team what they think about current discussion and they answer:

It looks like there are 2 requirements, first a plugin mechanism that satisfies what ng requires to acquire / and type information for ng's specific inferences. I do not believe swc as of today can offer both. There is in-progress work for plugin system at least on the rustlang side which someone may able to explore possibility though. For latter, I'm not honestly sure if ng wants full inferences for the type system. If that's the case, I expect that's related with swc's typechecker effort which is non trivial work to happen soon.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Dec 21, 2021

Thanks for the information. If swc can have a type checker similar to TypeScript, I believe Angular team would help to make efforts to make the current AST transformers to be able to use that.

The plan for Angular in the future would be still angular/angular#43131 which changes in the Angular internal architecture, which would simplify a lot efforts for 3rd party tools.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants