X Tutup
Skip to content

refactor(testing): move common testing logic into test_injector#5819

Closed
juliemr wants to merge 1 commit intoangular:masterfrom
juliemr:test-provider-config-3
Closed

refactor(testing): move common testing logic into test_injector#5819
juliemr wants to merge 1 commit intoangular:masterfrom
juliemr:test-provider-config-3

Conversation

@juliemr
Copy link
Copy Markdown
Member

@juliemr juliemr commented Dec 11, 2015

Before, all test framework wrappers (internal for dart and js/ts,
angular2_test for dart and testing for js/ts) had similar logic to
keep track of current global test injector and test provider list.
This change wraps that logic into one class managed by the test
injector.

@juliemr juliemr added the area: testing Issues related to Angular testing features, such as TestBed label Dec 11, 2015
@juliemr juliemr force-pushed the test-provider-config-3 branch 3 times, most recently from 9443a4c to 2f993ba Compare December 11, 2015 18:59
@juliemr juliemr added the action: review The PR is still awaiting reviews from at least one requested reviewer label Dec 11, 2015
@juliemr
Copy link
Copy Markdown
Member Author

juliemr commented Dec 11, 2015

@vicb arbitrarily assigning you as someone who's worked with testing :)

@juliemr
Copy link
Copy Markdown
Member Author

juliemr commented Dec 11, 2015

cc @kevmoo for a sanity check on the dart angular2_testing changes - this gets rid of reaching in to the test internals and just uses teardowns.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this final?

@kevmoo
Copy link
Copy Markdown
Contributor

kevmoo commented Dec 11, 2015

LGTM 😄

@juliemr juliemr force-pushed the test-provider-config-3 branch from 2f993ba to 7dd66a3 Compare December 11, 2015 20:03
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should _injector and _providers be denoted private ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done!

@juliemr juliemr force-pushed the test-provider-config-3 branch from 7dd66a3 to f66bfb7 Compare December 14, 2015 19:44
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make this private ?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Before, all test framework wrappers (internal for dart and js/ts,
angular2_test for dart and testing for js/ts) had similar logic to
keep track of current global test injector and test provider list.
This change wraps that logic into one class managed by the test
injector.
@juliemr juliemr force-pushed the test-provider-config-3 branch from f66bfb7 to 38c845f Compare December 14, 2015 20:54
@vicb vicb added pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Dec 14, 2015
@juliemr juliemr added the action: merge The PR is ready for merge by the caretaker label Dec 14, 2015
@mary-poppins
Copy link
Copy Markdown

Merging PR #5819 on behalf of @alexeagle to branch presubmit-alexeagle-pr-5819.

@angular-automatic-lock-bot
Copy link
Copy Markdown

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: testing Issues related to Angular testing features, such as TestBed cla: yes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants

X Tutup