FW-887: four independent fixes for TestBedRender3 #28033

alxhub · 2019-01-10T01:08:41Z

No description provided.

alxhub · 2019-01-10T01:11:07Z

Reviewers: please note the last two commits are the substance of this PR. The first commit is from #27860 which I expect to merge before this one, so I've based mine on it to avoid later conflicts.

mary-poppins · 2019-01-10T01:12:46Z

You can preview 0573fff at https://pr28033-0573fff.ngbuilds.io/.

AndrewKushnir · 2019-01-10T01:14:22Z

@alxhub PR #27860 that you based your PR on is now in master, so you can rebased from master branch. Thank you.

alxhub · 2019-01-10T16:34:40Z

@AndrewKushnir thanks, done.

mary-poppins · 2019-01-10T16:38:02Z

You can preview e1ba988 at https://pr28033-e1ba988.ngbuilds.io/.

mary-poppins · 2019-01-10T16:53:59Z

You can preview 4c48219 at https://pr28033-4c48219.ngbuilds.io/.

mary-poppins · 2019-01-10T17:15:46Z

You can preview a25022e at https://pr28033-a25022e.ngbuilds.io/.

mary-poppins · 2019-01-10T17:38:29Z

You can preview 54d3da9 at https://pr28033-54d3da9.ngbuilds.io/.

mary-poppins · 2019-01-10T17:47:40Z

You can preview 3bb4e2f at https://pr28033-3bb4e2f.ngbuilds.io/.

@NgModule

An @NgModule with an 'id' property has its type registered in a global map of modules by id. This happens during compilation of the module. In Ivy, modules are first compiled when the @NgModule decorator executes. In tests, they might be passed again through the TestBed's compiler, resulting in a second compilation and registration. Before this fix, this second registration would cause an error, as the id was previously registered. This commit makes the registration idempotent, so if the same module type is being registered for the same id then no error is thrown. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed.

@NgModule

An @NgModule with invalid provider declarations produces errors under normal circumstances. However, within the TestBed two small issues with provider overrides interfered with the correct production of these errors: 1. a 'null' provider object caused a premature crash when the TestBed attempted to check for a 'provide' property on it with hasOwnProperty(). 2. the array of providers would have an empty override array appended to it for each input provider, which would pollute the error messages produced down the line. This commit fixes both of these issues, by 1) checking for null and 2) filtering out the empty override arrays. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed.

Previously when testing code injected the Compiler, it received the top-level Compiler implementation defined in linker/compiler.ts (and governed by the __PRE_R3__ switch). Code running under the TestBed, however, should always use a TestBed-aware Compiler implementation. This commit adds such an implementation to the TestBedRender3, which passes compiled modules through the _compileNgModule() function. With this change, 3 formerly disabled router integration tests now pass. FW-855 #resolve

@NgModule

When an @NgModule decorator executes, the module is added to a queue in render3/jit/module.ts. Reading an ngComponentDef property causes this queue to be flushed, ensuring that the component gets the correct module scope applied. In before_each.ts, a global beforeEach is added to all Angular tests which calls TestBed.resetTestingModule() prior to running each test. This in turn clears the module compilation queue (which is correct behavior, as modules declared within the test should not leak outside of it via the queue). So far this is okay. But before the first test runs, the module compilation queue is full of modules declared in global scope. No definitions have been read, so no flushes of the queue have been triggered. The global beforeEach triggers a reset of the queue, aborting all of the in-progress global compilation, breaking those classes when they're later used in tests. This commit adds logic to TestBedRender3 to respect the state of the module queue before the TestBed is first initialized or reset. The queue is flushed prior to such an operation to ensure global compilation is allowed to finish properly. With this fix, a platform-server test now passes (previously the <my-child> element was not detected as a component, because the encompassing module never finished compilation. FW-887 #resolve

AndrewKushnir

LGTM, thanks @alxhub!

@NgModule

An @NgModule with invalid provider declarations produces errors under normal circumstances. However, within the TestBed two small issues with provider overrides interfered with the correct production of these errors: 1. a 'null' provider object caused a premature crash when the TestBed attempted to check for a 'provide' property on it with hasOwnProperty(). 2. the array of providers would have an empty override array appended to it for each input provider, which would pollute the error messages produced down the line. This commit fixes both of these issues, by 1) checking for null and 2) filtering out the empty override arrays. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close #28033

Previously when testing code injected the Compiler, it received the top-level Compiler implementation defined in linker/compiler.ts (and governed by the __PRE_R3__ switch). Code running under the TestBed, however, should always use a TestBed-aware Compiler implementation. This commit adds such an implementation to the TestBedRender3, which passes compiled modules through the _compileNgModule() function. With this change, 3 formerly disabled router integration tests now pass. FW-855 #resolve PR Close #28033

@NgModule

…les (#28033) When an @NgModule decorator executes, the module is added to a queue in render3/jit/module.ts. Reading an ngComponentDef property causes this queue to be flushed, ensuring that the component gets the correct module scope applied. In before_each.ts, a global beforeEach is added to all Angular tests which calls TestBed.resetTestingModule() prior to running each test. This in turn clears the module compilation queue (which is correct behavior, as modules declared within the test should not leak outside of it via the queue). So far this is okay. But before the first test runs, the module compilation queue is full of modules declared in global scope. No definitions have been read, so no flushes of the queue have been triggered. The global beforeEach triggers a reset of the queue, aborting all of the in-progress global compilation, breaking those classes when they're later used in tests. This commit adds logic to TestBedRender3 to respect the state of the module queue before the TestBed is first initialized or reset. The queue is flushed prior to such an operation to ensure global compilation is allowed to finish properly. With this fix, a platform-server test now passes (previously the <my-child> element was not detected as a component, because the encompassing module never finished compilation. FW-887 #resolve PR Close #28033

@NgModule

An @NgModule with an 'id' property has its type registered in a global map of modules by id. This happens during compilation of the module. In Ivy, modules are first compiled when the @NgModule decorator executes. In tests, they might be passed again through the TestBed's compiler, resulting in a second compilation and registration. Before this fix, this second registration would cause an error, as the id was previously registered. This commit makes the registration idempotent, so if the same module type is being registered for the same id then no error is thrown. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close angular#28033

@NgModule

…28033) An @NgModule with invalid provider declarations produces errors under normal circumstances. However, within the TestBed two small issues with provider overrides interfered with the correct production of these errors: 1. a 'null' provider object caused a premature crash when the TestBed attempted to check for a 'provide' property on it with hasOwnProperty(). 2. the array of providers would have an empty override array appended to it for each input provider, which would pollute the error messages produced down the line. This commit fixes both of these issues, by 1) checking for null and 2) filtering out the empty override arrays. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close angular#28033

) Previously when testing code injected the Compiler, it received the top-level Compiler implementation defined in linker/compiler.ts (and governed by the __PRE_R3__ switch). Code running under the TestBed, however, should always use a TestBed-aware Compiler implementation. This commit adds such an implementation to the TestBedRender3, which passes compiled modules through the _compileNgModule() function. With this change, 3 formerly disabled router integration tests now pass. FW-855 #resolve PR Close angular#28033

@NgModule

…les (angular#28033) When an @NgModule decorator executes, the module is added to a queue in render3/jit/module.ts. Reading an ngComponentDef property causes this queue to be flushed, ensuring that the component gets the correct module scope applied. In before_each.ts, a global beforeEach is added to all Angular tests which calls TestBed.resetTestingModule() prior to running each test. This in turn clears the module compilation queue (which is correct behavior, as modules declared within the test should not leak outside of it via the queue). So far this is okay. But before the first test runs, the module compilation queue is full of modules declared in global scope. No definitions have been read, so no flushes of the queue have been triggered. The global beforeEach triggers a reset of the queue, aborting all of the in-progress global compilation, breaking those classes when they're later used in tests. This commit adds logic to TestBedRender3 to respect the state of the module queue before the TestBed is first initialized or reset. The queue is flushed prior to such an operation to ensure global compilation is allowed to finish properly. With this fix, a platform-server test now passes (previously the <my-child> element was not detected as a component, because the encompassing module never finished compilation. FW-887 #resolve PR Close angular#28033

@NgModule

An @NgModule with an 'id' property has its type registered in a global map of modules by id. This happens during compilation of the module. In Ivy, modules are first compiled when the @NgModule decorator executes. In tests, they might be passed again through the TestBed's compiler, resulting in a second compilation and registration. Before this fix, this second registration would cause an error, as the id was previously registered. This commit makes the registration idempotent, so if the same module type is being registered for the same id then no error is thrown. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close angular#28033

@NgModule

…28033) An @NgModule with invalid provider declarations produces errors under normal circumstances. However, within the TestBed two small issues with provider overrides interfered with the correct production of these errors: 1. a 'null' provider object caused a premature crash when the TestBed attempted to check for a 'provide' property on it with hasOwnProperty(). 2. the array of providers would have an empty override array appended to it for each input provider, which would pollute the error messages produced down the line. This commit fixes both of these issues, by 1) checking for null and 2) filtering out the empty override arrays. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close angular#28033

) Previously when testing code injected the Compiler, it received the top-level Compiler implementation defined in linker/compiler.ts (and governed by the __PRE_R3__ switch). Code running under the TestBed, however, should always use a TestBed-aware Compiler implementation. This commit adds such an implementation to the TestBedRender3, which passes compiled modules through the _compileNgModule() function. With this change, 3 formerly disabled router integration tests now pass. FW-855 #resolve PR Close angular#28033

@NgModule

…les (angular#28033) When an @NgModule decorator executes, the module is added to a queue in render3/jit/module.ts. Reading an ngComponentDef property causes this queue to be flushed, ensuring that the component gets the correct module scope applied. In before_each.ts, a global beforeEach is added to all Angular tests which calls TestBed.resetTestingModule() prior to running each test. This in turn clears the module compilation queue (which is correct behavior, as modules declared within the test should not leak outside of it via the queue). So far this is okay. But before the first test runs, the module compilation queue is full of modules declared in global scope. No definitions have been read, so no flushes of the queue have been triggered. The global beforeEach triggers a reset of the queue, aborting all of the in-progress global compilation, breaking those classes when they're later used in tests. This commit adds logic to TestBedRender3 to respect the state of the module queue before the TestBed is first initialized or reset. The queue is flushed prior to such an operation to ensure global compilation is allowed to finish properly. With this fix, a platform-server test now passes (previously the <my-child> element was not detected as a component, because the encompassing module never finished compilation. FW-887 #resolve PR Close angular#28033

@NgModule

An @NgModule with an 'id' property has its type registered in a global map of modules by id. This happens during compilation of the module. In Ivy, modules are first compiled when the @NgModule decorator executes. In tests, they might be passed again through the TestBed's compiler, resulting in a second compilation and registration. Before this fix, this second registration would cause an error, as the id was previously registered. This commit makes the registration idempotent, so if the same module type is being registered for the same id then no error is thrown. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close angular#28033

@NgModule

…28033) An @NgModule with invalid provider declarations produces errors under normal circumstances. However, within the TestBed two small issues with provider overrides interfered with the correct production of these errors: 1. a 'null' provider object caused a premature crash when the TestBed attempted to check for a 'provide' property on it with hasOwnProperty(). 2. the array of providers would have an empty override array appended to it for each input provider, which would pollute the error messages produced down the line. This commit fixes both of these issues, by 1) checking for null and 2) filtering out the empty override arrays. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close angular#28033

) Previously when testing code injected the Compiler, it received the top-level Compiler implementation defined in linker/compiler.ts (and governed by the __PRE_R3__ switch). Code running under the TestBed, however, should always use a TestBed-aware Compiler implementation. This commit adds such an implementation to the TestBedRender3, which passes compiled modules through the _compileNgModule() function. With this change, 3 formerly disabled router integration tests now pass. FW-855 #resolve PR Close angular#28033

@NgModule

…les (angular#28033) When an @NgModule decorator executes, the module is added to a queue in render3/jit/module.ts. Reading an ngComponentDef property causes this queue to be flushed, ensuring that the component gets the correct module scope applied. In before_each.ts, a global beforeEach is added to all Angular tests which calls TestBed.resetTestingModule() prior to running each test. This in turn clears the module compilation queue (which is correct behavior, as modules declared within the test should not leak outside of it via the queue). So far this is okay. But before the first test runs, the module compilation queue is full of modules declared in global scope. No definitions have been read, so no flushes of the queue have been triggered. The global beforeEach triggers a reset of the queue, aborting all of the in-progress global compilation, breaking those classes when they're later used in tests. This commit adds logic to TestBedRender3 to respect the state of the module queue before the TestBed is first initialized or reset. The queue is flushed prior to such an operation to ensure global compilation is allowed to finish properly. With this fix, a platform-server test now passes (previously the <my-child> element was not detected as a component, because the encompassing module never finished compilation. FW-887 #resolve PR Close angular#28033

@NgModule

An @NgModule with an 'id' property has its type registered in a global map of modules by id. This happens during compilation of the module. In Ivy, modules are first compiled when the @NgModule decorator executes. In tests, they might be passed again through the TestBed's compiler, resulting in a second compilation and registration. Before this fix, this second registration would cause an error, as the id was previously registered. This commit makes the registration idempotent, so if the same module type is being registered for the same id then no error is thrown. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close angular#28033

@NgModule

…28033) An @NgModule with invalid provider declarations produces errors under normal circumstances. However, within the TestBed two small issues with provider overrides interfered with the correct production of these errors: 1. a 'null' provider object caused a premature crash when the TestBed attempted to check for a 'provide' property on it with hasOwnProperty(). 2. the array of providers would have an empty override array appended to it for each input provider, which would pollute the error messages produced down the line. This commit fixes both of these issues, by 1) checking for null and 2) filtering out the empty override arrays. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close angular#28033

) Previously when testing code injected the Compiler, it received the top-level Compiler implementation defined in linker/compiler.ts (and governed by the __PRE_R3__ switch). Code running under the TestBed, however, should always use a TestBed-aware Compiler implementation. This commit adds such an implementation to the TestBedRender3, which passes compiled modules through the _compileNgModule() function. With this change, 3 formerly disabled router integration tests now pass. FW-855 #resolve PR Close angular#28033

@NgModule

…les (angular#28033) When an @NgModule decorator executes, the module is added to a queue in render3/jit/module.ts. Reading an ngComponentDef property causes this queue to be flushed, ensuring that the component gets the correct module scope applied. In before_each.ts, a global beforeEach is added to all Angular tests which calls TestBed.resetTestingModule() prior to running each test. This in turn clears the module compilation queue (which is correct behavior, as modules declared within the test should not leak outside of it via the queue). So far this is okay. But before the first test runs, the module compilation queue is full of modules declared in global scope. No definitions have been read, so no flushes of the queue have been triggered. The global beforeEach triggers a reset of the queue, aborting all of the in-progress global compilation, breaking those classes when they're later used in tests. This commit adds logic to TestBedRender3 to respect the state of the module queue before the TestBed is first initialized or reset. The queue is flushed prior to such an operation to ensure global compilation is allowed to finish properly. With this fix, a platform-server test now passes (previously the <my-child> element was not detected as a component, because the encompassing module never finished compilation. FW-887 #resolve PR Close angular#28033

@NgModule

An @NgModule with an 'id' property has its type registered in a global map of modules by id. This happens during compilation of the module. In Ivy, modules are first compiled when the @NgModule decorator executes. In tests, they might be passed again through the TestBed's compiler, resulting in a second compilation and registration. Before this fix, this second registration would cause an error, as the id was previously registered. This commit makes the registration idempotent, so if the same module type is being registered for the same id then no error is thrown. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close angular#28033

@NgModule

…28033) An @NgModule with invalid provider declarations produces errors under normal circumstances. However, within the TestBed two small issues with provider overrides interfered with the correct production of these errors: 1. a 'null' provider object caused a premature crash when the TestBed attempted to check for a 'provide' property on it with hasOwnProperty(). 2. the array of providers would have an empty override array appended to it for each input provider, which would pollute the error messages produced down the line. This commit fixes both of these issues, by 1) checking for null and 2) filtering out the empty override arrays. Testing strategy: future commits change the way the TestBed compiles modules, causing tests to become sensitive to this bug if not fixed. PR Close angular#28033

) Previously when testing code injected the Compiler, it received the top-level Compiler implementation defined in linker/compiler.ts (and governed by the __PRE_R3__ switch). Code running under the TestBed, however, should always use a TestBed-aware Compiler implementation. This commit adds such an implementation to the TestBedRender3, which passes compiled modules through the _compileNgModule() function. With this change, 3 formerly disabled router integration tests now pass. FW-855 #resolve PR Close angular#28033

@NgModule

…les (angular#28033) When an @NgModule decorator executes, the module is added to a queue in render3/jit/module.ts. Reading an ngComponentDef property causes this queue to be flushed, ensuring that the component gets the correct module scope applied. In before_each.ts, a global beforeEach is added to all Angular tests which calls TestBed.resetTestingModule() prior to running each test. This in turn clears the module compilation queue (which is correct behavior, as modules declared within the test should not leak outside of it via the queue). So far this is okay. But before the first test runs, the module compilation queue is full of modules declared in global scope. No definitions have been read, so no flushes of the queue have been triggered. The global beforeEach triggers a reset of the queue, aborting all of the in-progress global compilation, breaking those classes when they're later used in tests. This commit adds logic to TestBedRender3 to respect the state of the module queue before the TestBed is first initialized or reset. The queue is flushed prior to such an operation to ensure global compilation is allowed to finish properly. With this fix, a platform-server test now passes (previously the <my-child> element was not detected as a component, because the encompassing module never finished compilation. FW-887 #resolve PR Close angular#28033

angular-automatic-lock-bot · 2019-09-14T12:52:58Z

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.}

alxhub requested review from a team as code owners January 10, 2019 01:08

googlebot added the cla: yes label Jan 10, 2019

alxhub added action: review The PR is still awaiting reviews from at least one requested reviewer target: major This PR is targeted for the next major release labels Jan 10, 2019

alxhub requested review from jasonaden and kara January 10, 2019 01:09

alxhub force-pushed the jit-ngmodule-compilation branch from 0573fff to e1ba988 Compare January 10, 2019 16:33

alxhub force-pushed the jit-ngmodule-compilation branch from e1ba988 to 4c48219 Compare January 10, 2019 16:49

alxhub force-pushed the jit-ngmodule-compilation branch from 4c48219 to a25022e Compare January 10, 2019 17:11

alxhub force-pushed the jit-ngmodule-compilation branch from a25022e to 54d3da9 Compare January 10, 2019 17:33

alxhub force-pushed the jit-ngmodule-compilation branch from 54d3da9 to 3bb4e2f Compare January 10, 2019 17:43

alxhub requested review from AndrewKushnir and removed request for kara January 10, 2019 17:50

alxhub added 4 commits January 10, 2019 13:53

alxhub force-pushed the jit-ngmodule-compilation branch from 3bb4e2f to 975257a Compare January 10, 2019 22:01

alxhub changed the title ~~FW-887: two independent fixes for TestBedRender3~~ FW-887: four independent fixes for TestBedRender3 Jan 10, 2019

AndrewKushnir approved these changes Jan 11, 2019

View reviewed changes

alxhub added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 11, 2019

alxhub removed the request for review from jasonaden January 11, 2019 18:29

AndrewKushnir closed this in 6c6d430 Jan 11, 2019

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FW-887: four independent fixes for TestBedRender3 #28033

FW-887: four independent fixes for TestBedRender3 #28033

alxhub commented Jan 10, 2019

alxhub commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

AndrewKushnir commented Jan 10, 2019

alxhub commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

AndrewKushnir left a comment

angular-automatic-lock-bot bot commented Sep 14, 2019

FW-887: four independent fixes for TestBedRender3 #28033

FW-887: four independent fixes for TestBedRender3 #28033

Conversation

alxhub commented Jan 10, 2019

alxhub commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

AndrewKushnir commented Jan 10, 2019

alxhub commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

mary-poppins commented Jan 10, 2019

AndrewKushnir left a comment

Choose a reason for hiding this comment

angular-automatic-lock-bot bot commented Sep 14, 2019