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

TestBed.overrideModule affects other tests #36619

Closed
kalsowerus opened this issue Apr 14, 2020 · 5 comments
Closed

TestBed.overrideModule affects other tests #36619

kalsowerus opened this issue Apr 14, 2020 · 5 comments
Assignees
Labels
area: core Issues related to the framework runtime area: testing Issues related to Angular testing features, such as TestBed freq2: medium regression Indicates than the issue relates to something that worked in a previous version state: has PR type: bug/fix
Milestone

Comments

@kalsowerus
Copy link

🐞 bug report

Affected Package

The issue is caused by package @angular/core/testing

Is this a regression?

Yes, the previous version in which this bug was not present was: 8.2.13

Description

All test cases after a call to TestBed.overrideModule seem to be affected by its modifications.

In the provided Example project I have two tests: one.spec.ts and two.spec.ts. Both create an instance of their own component which both contain a TestComponent. In one.spec.ts I replace TestComponent with a Mock using overrideModule. If one.spec.ts is executed first by the test runner, two.spec.ts also uses the Mock component.

Additionally this example also does not work if two.spec.ts is executed first. In that case the overrideModule-call seems to be ignored.

I cannot say for sure wether this is a problem with Angular or Karma. If you think I should open an issue on Karma, please let me know.

🔬 Minimal Reproduction

https://github.com/kalsowerus/repro-app

The TestComponent I mentioned earlier prints 'test' in its ngOnInit(), while the mock component inside one.spec.ts prints 'mock'. To see the problem just npm install and then ng test. The exact command I used to execute the tests is ng test --browsers ChromeForWSL --watch=false. ChromeForWSL being a custom Launcher based on Chrome which disables the "VizDisplayCompositor"-feature since that does not seem to work in a WSL (Windows subsystem for Linux) environment.

The expected result would be seeing both 'test' and 'mock' in the console once in any order. But depending on the execution order either of them are printed twice while the other one is not printed at all.

🔥 Exception or Error

No exception or error

🌍 Your Environment

Angular Version:


Angular CLI: 9.1.1
Node: 12.14.0
OS: linux x64

Angular: 9.1.1
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router
Ivy Workspace: Yes

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.901.1
@angular-devkit/build-angular     0.901.1
@angular-devkit/build-optimizer   0.901.1
@angular-devkit/build-webpack     0.901.1
@angular-devkit/core              9.1.1
@angular-devkit/schematics        9.1.1
@ngtools/webpack                  9.1.1
@schematics/angular               9.1.1
@schematics/update                0.901.1
rxjs                              6.5.5
typescript                        3.8.3
webpack                           4.42.0

Anything else relevant?

@AndrewKushnir AndrewKushnir self-assigned this Apr 14, 2020
@AndrewKushnir AndrewKushnir added area: core Issues related to the framework runtime comp: ivy area: testing Issues related to Angular testing features, such as TestBed labels Apr 14, 2020
@ngbot ngbot bot added this to the needsTriage milestone Apr 14, 2020
@AndrewKushnir
Copy link
Contributor

Hi @kalsowerus,

Thanks for reporting the issue and providing a repro (this is super helpful!).

I was able to reproduce the problem, will perform further investigation and will keep this thread updated.

Thank you.

@AndrewKushnir AndrewKushnir added freq2: medium regression Indicates than the issue relates to something that worked in a previous version type: bug/fix labels Apr 15, 2020
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Apr 15, 2020
AndrewKushnir added a commit to AndrewKushnir/angular that referenced this issue Apr 15, 2020
When module overrides (via `TestBed.overrideModule`) are present, it might affect all modules that import (even transitively) an overridden one. For all affected modules we need to recalculate their scopes for a given test run and restore original scopes at the end. Prior to this change, we were recalculating module scopes only for components that are used in a test, without taking into account module hierarchy. This commit updates Ivy TestBed logic to calculate all potentially affected modules are reset cached scopes information for them (so that scopes are recalculated as needed).

Resolves angular#36619.
AndrewKushnir added a commit to AndrewKushnir/angular that referenced this issue Apr 20, 2020
When module overrides (via `TestBed.overrideModule`) are present, it might affect all modules that import (even transitively) an overridden one. For all affected modules we need to recalculate their scopes for a given test run and restore original scopes at the end. Prior to this change, we were recalculating module scopes only for components that are used in a test, without taking into account module hierarchy. This commit updates Ivy TestBed logic to calculate all potentially affected modules are reset cached scopes information for them (so that scopes are recalculated as needed).

Resolves angular#36619.
@AndrewKushnir
Copy link
Contributor

Hi @kalsowerus,

I've created a PR with proposed fix. I tested the fix using the repro that you provided and it seems to be working correctly now. If you get a chance, it'd be great if you could help test this fix with your real app to see if the original problem is also resolved.

To test your app with the changes from PR #36649, you can follow an instruction in this document on how to use PR artifacts. An archive with updated @angular/core package that contains changes from the PR is available here (please make sure you update other @angular packages to 9.1.2 to avoid version conflicts). Note: the mentioned archive with updated @angular/core package is for testing purposes only and not suitable for production usage (that archive will be removed once PR is closed).

Thank you.

@kalsowerus
Copy link
Author

Hi @AndrewKushnir,

Thank you very much for your response. I ran my tests a few times with your PR artifact and could not reproduce the original error. It seems to be working fine now.

Can I expect to see this fix in 9.1.3?

Thank you.

AndrewKushnir added a commit to AndrewKushnir/angular that referenced this issue Apr 21, 2020
When module overrides (via `TestBed.overrideModule`) are present, it might affect all modules that import (even transitively) an overridden one. For all affected modules we need to recalculate their scopes for a given test run and restore original scopes at the end. Prior to this change, we were recalculating module scopes only for components that are used in a test, without taking into account module hierarchy. This commit updates Ivy TestBed logic to calculate all potentially affected modules are reset cached scopes information for them (so that scopes are recalculated as needed).

Resolves angular#36619.
@AndrewKushnir
Copy link
Contributor

Hi @kalsowerus, thanks for your help with testing the change 👍I expect the change to be included into 9.1.3 or 9.1.4 release. Thank you.

matsko pushed a commit that referenced this issue Apr 22, 2020
…#36649)

When module overrides (via `TestBed.overrideModule`) are present, it might affect all modules that import (even transitively) an overridden one. For all affected modules we need to recalculate their scopes for a given test run and restore original scopes at the end. Prior to this change, we were recalculating module scopes only for components that are used in a test, without taking into account module hierarchy. This commit updates Ivy TestBed logic to calculate all potentially affected modules are reset cached scopes information for them (so that scopes are recalculated as needed).

Resolves #36619.

PR Close #36649
@matsko matsko closed this as completed in 942b986 Apr 22, 2020
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators May 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: core Issues related to the framework runtime area: testing Issues related to Angular testing features, such as TestBed freq2: medium regression Indicates than the issue relates to something that worked in a previous version state: has PR type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants