Skip to content

/angular

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

feat(upgrade): provide test helpers for wiring up injectors #16848

Closed
wants to merge 3 commits into from
+516 −11

Conversation

Reviewers

@juliemr juliemr

@fzxt fzxt

@gkalpak gkalpak

@calebeno calebeno

@IgorMinar IgorMinar

@alxhub alxhub

Assignees

@petebacondarwin petebacondarwin

Labels
PR action: merge PR action: merge-assistance PR target: master-only cla: yes comp: upgrade type: feature
Projects
None yet
Milestone
  needsTriage
@petebacondarwin
Copy link
Member

commented May 17, 2017 •
edited

Sits on top of #29466

This PR provides two test helpers for the upgrade/static library, which wire up the Angular and AngularJS injectors without the need for a full bootstrap of a hybrid app.

  • createAngularJSTestingModule() should be used in AngularJS tests
  • createAngularTestingModule() should be used in Angular tests

Check out the spec files for example usage.

@petebacondarwin petebacondarwin added the comp: upgrade/static label May 17, 2017

@googlebot googlebot added the cla: yes label May 17, 2017

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngupgrade-testing-api branch from bfe4205 to c73b392 May 17, 2017

@mhevery

This comment has been minimized.

Sign in to view
Copy link
Member

commented May 17, 2017

Looking good! I think we need some more documentation on how to use this. Can this be part of the API docs?
@petebacondarwin

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented May 17, 2017

I'll add docs but wanted feedback on whether this is the right API first.
@juliemr

juliemr reviewed May 17, 2017
View changes

Show resolved Hide resolved packages/upgrade/test/testing/create_angular_test_module_spec.ts Outdated
@gkalpak

gkalpak reviewed May 22, 2017
View changes

Show resolved Hide resolved packages/upgrade/src/testing/create_angular_test_module.ts Outdated
Show resolved Hide resolved packages/upgrade/src/testing/create_angularjs_test_module.ts Outdated
Show resolved Hide resolved packages/upgrade/test/testing/create_angularjs_test_module_spec.ts Outdated
@gkalpak

This comment has been minimized.

Sign in to view
Copy link
Member

commented May 22, 2017

As a reminder, the helpers need to be exported and upgrade/testing should be added to a bunch of lists for it to appear in the docs etc.
@juliemr

This comment has been minimized.

Sign in to view
Copy link
Member

commented Jun 23, 2017

Ping?
@petebacondarwin

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jun 25, 2017

Sorry I have been distracted. I will tidy it up this week.

@petebacondarwin petebacondarwin self-assigned this Jun 25, 2017

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngupgrade-testing-api branch from c73b392 to 52735ee Jul 3, 2017

@petebacondarwin

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jul 3, 2017

I have made the suggested changes, where appropriate, added documentation and included this new module in as many configuration files as I could find.

PTAL: @mhevery @juliemr @gkalpak
@petebacondarwin

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jul 3, 2017

And of course this PR falls foul of the change from CircleCI v1 to v2.

@petebacondarwin petebacondarwin closed this Jul 3, 2017

@petebacondarwin petebacondarwin reopened this Jul 3, 2017

@petebacondarwin

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jul 3, 2017

Just checking to see if closing and reopening helps with CircleCI

@gkalpak gkalpak referenced this pull request Jul 4, 2017

Closed

Angular 1 unit tests fail with Angular 2 components using downgradeComponents #14263

@gkalpak

gkalpak reviewed Jul 5, 2017
View changes

Copy link
Member

left a comment

Just a few nits. Looks great so far (except that I have no idea about all the places we need to list a new package 😁)
I assume more tests will be added when this is finalized.

One thing I am a little concerned with is the namespacing of the module. Typically @angular/foo/testing is for testing @angular/foo/bar. So, I would expect the testing package for @angular/upgrade/static to be @angular/upgrade/static/testing (while @angular/upgrade/testing sounds more about testing @angular/upgrade).
I hope that eventually, @angular/upgrade will get dropped and @angular/upgrade/static will be moved to @angular/upgrade (together with its testing module).
Show resolved Hide resolved aio/tools/transforms/angular-api-package/index.js Outdated
integration/typings_test_ts21/include-all.ts Outdated
@@ -21,6 +21,7 @@ import * as platformServerTesting from '@angular/platform-server/testing';
import * as router from '@angular/router';
import * as routerTesting from '@angular/router/testing';
import * as upgrade from '@angular/upgrade';

This comment has been minimized.

Sign in to view
Copy link
@gkalpak

gkalpak Jul 5, 2017

Member

Unrelated to this PR: Does this include upgrade/static? Shouldn't it be explicitly imported?
Show resolved Hide resolved packages/upgrade/testing/src/create_angular_test_module.ts Outdated
Show resolved Hide resolved packages/upgrade/testing/src/create_angularjs_test_module.ts Outdated
Show resolved Hide resolved packages/upgrade/testing/src/create_angularjs_test_module.ts Outdated
Show resolved Hide resolved packages/upgrade/testing/src/create_angularjs_test_module.ts Outdated
Show resolved Hide resolved packages/upgrade/testing/src/create_angularjs_test_module.ts Outdated
Show resolved Hide resolved packages/upgrade/testing/src/create_angular_test_module.ts Outdated

@mhevery mhevery force-pushed the angular:master branch from 08b36cb to 5d4b36f Jul 20, 2017

@kieranblight

This comment has been minimized.

Sign in to view
Copy link

commented Jul 21, 2017 •
edited

Hi @petebacondarwin, I am just wondering what the rough timeline for this to be release will be?

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngupgrade-testing-api branch from 0743841 to 0260634 Jul 22, 2017

@petebacondarwin

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jul 22, 2017

@kieranblight - I have been having difficulties getting the new package into the system. I hope to finish this and get it in by the end of this week.
@kieranblight

This comment has been minimized.

Sign in to view
Copy link

commented Jul 24, 2017

@petebacondarwin thank you, look forward to it :)!
@juliemr

juliemr reviewed Jul 25, 2017
View changes

Show resolved Hide resolved packages/upgrade/testing/src/create_angular_test_module.ts Outdated

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngupgrade-testing-api branch 3 times, most recently from 0ca84ba to 733d0aa Jul 29, 2017

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngupgrade-testing-api branch 2 times, most recently from d3c0ab5 to 590e8c2 Aug 16, 2017

@petebacondarwin petebacondarwin added PR action: review and removed PR state: WIP labels Apr 13, 2019

@fzxt

This comment has been minimized.

Sign in to view
Copy link

commented Apr 20, 2019

Ping? It would be nice to get these helpers in soon. Can any googlers help out with the review?
@petebacondarwin

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Apr 20, 2019

@jasonaden is working on it :-)

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngupgrade-testing-api branch from eac230b to 5d53a7e Apr 25, 2019

@zuzusik

This comment has been minimized.

Sign in to view
Copy link

commented Jun 13, 2019

any expectations on when this is going to be finished?

thanks!
@petebacondarwin

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jun 13, 2019

Apart from the conflicts it is ready to be reviewed. It is unlikely to land before 8.2 or 9 now.
I'll rebase tomorrow and chase someone (like @IgorMinar) to take a view.

petebacondarwin added some commits Mar 22, 2019

@petebacondarwin
fix(compiler): stringify `Object.create(null)` tokens
Loading status checks…
113e582
@petebacondarwin
feat(upgrade): provide unit test helpers for wiring up injectors 
Adds two new helper functions that can be used when unit testing Angular services
that depend upon upgraded AngularJS services, or vice versa.
The functions return a module (AngularJS or NgModule) that is configured to wire up
the Angular and AngularJS injectors without the need to actually bootstrap a full
hybrid application.

This makes it simpler and faster to unit test services.
Loading status checks…
c2752af

@petebacondarwin petebacondarwin force-pushed the petebacondarwin:ngupgrade-testing-api branch from 5d53a7e to c2752af Jun 14, 2019

@petebacondarwin petebacondarwin added PR target: master-only and removed PR target: TBD labels Jun 14, 2019

@gkalpak

gkalpak approved these changes Jun 14, 2019
View changes

Show resolved Hide resolved packages/examples/upgrade/static/ts/full/module.ts Outdated
Show resolved Hide resolved packages/upgrade/static/testing/BUILD.bazel Outdated
Show resolved Hide resolved ...ages/upgrade/static/testing/test/create_angularjs_testing_module_spec.ts Outdated
Show resolved Hide resolved packages/upgrade/static/testing/tsconfig-build.json Outdated
@alxhub

alxhub approved these changes Jun 14, 2019
View changes

Copy link
Contributor

left a comment

+1 for compiler.
@IgorMinar

IgorMinar approved these changes Jun 17, 2019
View changes

Copy link
Member

left a comment

Lgtm for API changes.

@petebacondarwin petebacondarwin added PR action: merge PR action: presubmit and removed PR action: review labels Jun 17, 2019

@AndrewKushnir

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Jun 17, 2019

VE presubmit

@AndrewKushnir AndrewKushnir added the PR action: cleanup label Jun 17, 2019

@AndrewKushnir

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Jun 17, 2019

Presubmit indicated that we need to perform additional changes in build files in g3 to land this PR. I've added "cleanup" label for now and will look into the build files update asap. Thank you.
@petebacondarwin

This comment has been minimized.

Sign in to view
Copy link
Member Author

commented Jun 18, 2019

Thanks @AndrewKushnir
@AndrewKushnir

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented Jun 19, 2019

New VE presubmit, including g3 changes.

Note to Caretaker: this PR requires additional change in g3: cl/254085499, to update BUILD file with extra rules to build upgrade/testing package.

@AndrewKushnir AndrewKushnir added PR action: merge-assistance and removed PR action: cleanup PR action: presubmit labels Jun 19, 2019

@kara kara closed this in 5e53956 Jun 21, 2019

kara added a commit that referenced this pull request Jun 21, 2019

@petebacondarwin @kara
feat(upgrade): provide unit test helpers for wiring up injectors (#16848 
)

Adds two new helper functions that can be used when unit testing Angular services
that depend upon upgraded AngularJS services, or vice versa.
The functions return a module (AngularJS or NgModule) that is configured to wire up
the Angular and AngularJS injectors without the need to actually bootstrap a full
hybrid application.

This makes it simpler and faster to unit test services.

PR Close #16848
Loading status checks…
3fb78aa

@iget-master iget-master referenced this pull request Jul 10, 2019

Open

CHANGELOG.md shouldn't include pre-release changes? #31491

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.