fix(docs-infra): speed up ng test #39866
Conversation
| @@ -14,6 +15,12 @@ getTestBed().initTestEnvironment( | |||
| BrowserDynamicTestingModule, | |||
| platformBrowserDynamicTesting() | |||
| ); | |||
|
|
|||
| // https://github.com/angular/angular/issues/31834 | |||
There was a problem hiding this comment.
| // https://github.com/angular/angular/issues/31834 | |
| // TODO: Remove this workaround once we update to an Angular version that includes a fix for | |
| // https://github.com/angular/angular/issues/31834. |
There is currently a known issue with the framework when testing that leaks style. See angular#31834 A possible temporary workaround is to manually call the service that adds/aremoves the style. See angular#31834 (comment) This workaround will be unnecessary when angular#38336 lands. This speeds up the `ng test` task locally by ~30%: - before: Executed 649 of 652 (skipped 3) SUCCESS (24.161 secs / 20.072 secs) - after: Executed 649 of 652 (skipped 3) SUCCESS (17.854 secs / 15.584 secs)
24e3b01 to
da0a7f5
Compare
There was a problem hiding this comment.
I concur with @gkalpak - I see no real difference with and without this change.
Before:
Executed 649 of 652 (skipped 3) SUCCESS (19.422 secs / 16.503 secs)
Executed 649 of 652 (skipped 3) SUCCESS (19.22 secs / 16.283 secs)
Executed 649 of 652 (skipped 3) SUCCESS (19.539 secs / 16.699 secs)
After:
Executed 649 of 652 (skipped 3) SUCCESS (19.021 secs / 16.243 secs)
Executed 649 of 652 (skipped 3) SUCCESS (19.738 secs / 16.692 secs)
Executed 649 of 652 (skipped 3) SUCCESS (20.074 secs / 17.067 secs)
Note this is running Chrome Headless 84.0.4147.0 (Mac OS 10.16.0
Perhaps the styles issue is not present on a headless browser?
@cexbrayat - can you look into why/how you get the improvement?
If we cannot show a clear improvement then I would not be inclined to land this since it is already a workaround/hack in lieu of the full fix.
|
Strange, I'm also on MacOS with headless Chome 84 🤔 The "real life" projects we work on saw a 2x-3x improvement as there are thousands of tests. It's also more visible on CI as I guess the memory pressure is more important on smaller machines (and maybe that's the difference between us: I have a fairly old laptop). |
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
There is currently a known issue with the framework when testing that leaks style.
See #31834
A possible temporary workaround is to manually call the service that adds/aremoves the style.
See #31834 (comment)
This workaround will be unnecessary when #38336 lands.
What is the new behavior?
This speeds up the
ng testtask locally by ~30%:Does this PR introduce a breaking change?