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

fix(core): align TestBed interfaces and implementation #46635

Closed
wants to merge 4 commits into from

Conversation

AndrewKushnir
Copy link
Contributor

This PR contains 3 commits with the TestBed implementation cleanup.

The change affected the public API surface, but in practice those types were already exposed via the TestBedStatic interface, so those APIs were already public.

PR Type

What kind of change does this PR introduce?

  • Refactoring (no functional changes, no api changes)

Does this PR introduce a breaking change?

  • Yes
  • No

@AndrewKushnir AndrewKushnir added state: WIP area: testing Issues related to Angular testing features, such as TestBed target: patch This PR is targeted for the next patch release labels Jun 30, 2022
@ngbot ngbot bot added this to the Backlog milestone Jun 30, 2022
@AndrewKushnir AndrewKushnir modified the milestones: Backlog, Fixit H1'2022 Jun 30, 2022
@AndrewKushnir
Copy link
Contributor Author

AndrewKushnir commented Jun 30, 2022

Presubmit (this change will also require a TGP) + a local patch in g3.

@AndrewKushnir AndrewKushnir marked this pull request as ready for review June 30, 2022 01:40
@AndrewKushnir AndrewKushnir added action: review The PR is still awaiting reviews from at least one requested reviewer and removed state: WIP labels Jun 30, 2022
@pullapprove pullapprove bot requested review from atscott and dylhunn June 30, 2022 01:40
Copy link
Contributor

@atscott atscott left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

@pullapprove pullapprove bot requested a review from jessicajaniuk June 30, 2022 14:32
Copy link
Contributor

@dylhunn dylhunn left a comment

Choose a reason for hiding this comment

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

reviewed-for: public-api

Copy link
Contributor

@jessicajaniuk jessicajaniuk left a comment

Choose a reason for hiding this comment

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

LGTM

reviewed-for: fw-core, fw-testing, circular-dependencies, public-api

@jessicajaniuk jessicajaniuk removed the request for review from alxhub June 30, 2022 22:09
@AndrewKushnir AndrewKushnir force-pushed the testbed_refactor branch 2 times, most recently from 9fa5f01 to 5301581 Compare June 30, 2022 22:48
@pullapprove pullapprove bot requested a review from alxhub June 30, 2022 22:48
@angular angular deleted a comment from mary-poppins Jul 1, 2022
@AndrewKushnir
Copy link
Contributor Author

Global Presubmit.

@AndrewKushnir AndrewKushnir changed the title refactor(core): cleanup TestBed implementation and interfaces fix(core): align TestBed interfaces and implementation Jul 1, 2022
@AndrewKushnir AndrewKushnir force-pushed the testbed_refactor branch 2 times, most recently from 5e78262 to 217a3dc Compare July 2, 2022 00:20
This commit performs various refactoring of the TestBed code to better align interfaces and implementation. The implementation class is also renamed from `TestBedRender3` -> `TestBedImpl`, but the public API name has not changed.

Note: as a part of this change, the TestBed interface became more consistent and typings for multiple methods were updated to account for the fact that the TestBed reference is returned. This was always a runtime behavior of TestBed, which was not reflected in few places in type.
This commit renames the `R3TestBedCompiler` -> `TestBedCompiler` to drop no longer needed Render3 prefix.
This commit updates TestBed-related files to drop the `r3_` prefix.
… cycles

This commit combines TestBed interface and implementation to avoid cycles in the import graph. There are no functional or API changes.
@angular angular deleted a comment from mary-poppins Jul 8, 2022
@angular angular deleted a comment from mary-poppins Jul 8, 2022
@angular angular deleted a comment from mary-poppins Jul 8, 2022
@angular angular deleted a comment from mary-poppins Jul 8, 2022
@AndrewKushnir AndrewKushnir removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Jul 8, 2022
@mary-poppins
Copy link

You can preview bd2b7e8 at https://pr46635-bd2b7e8.ngbuilds.io/.

@AndrewKushnir AndrewKushnir added the action: global presubmit The PR is in need of a google3 global presubmit label Jul 8, 2022
@AndrewKushnir
Copy link
Contributor Author

Global Presubmit #2.

@AndrewKushnir
Copy link
Contributor Author

Global Presubmit #3 (to make sure there are no new regressions).

@AndrewKushnir AndrewKushnir added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jul 12, 2022
Copy link
Member

@alxhub alxhub left a comment

Choose a reason for hiding this comment

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

Reviewed-for: public-api

@pullapprove pullapprove bot requested a review from alxhub July 13, 2022 08:23
@AndrewKushnir AndrewKushnir added action: merge The PR is ready for merge by the caretaker merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note and removed action: global presubmit The PR is in need of a google3 global presubmit labels Jul 13, 2022
@AndrewKushnir
Copy link
Contributor Author

AndrewKushnir commented Jul 13, 2022

Merge assistance: the PR has a "green" TGP, but it would still make sense to merge and land it in g3 separately from other changes.

Regarding the PullApprove status: there is enough approvals from the group, so we can proceed with merging this PR.

@jessicajaniuk
Copy link
Contributor

This PR was merged into the repository by commit 7e03fc9.

jessicajaniuk pushed a commit that referenced this pull request Jul 13, 2022
This commit renames the `R3TestBedCompiler` -> `TestBedCompiler` to drop no longer needed Render3 prefix.

PR Close #46635
jessicajaniuk pushed a commit that referenced this pull request Jul 13, 2022
#46635)

This commit updates TestBed-related files to drop the `r3_` prefix.

PR Close #46635
jessicajaniuk pushed a commit that referenced this pull request Jul 13, 2022
… cycles (#46635)

This commit combines TestBed interface and implementation to avoid cycles in the import graph. There are no functional or API changes.

PR Close #46635
@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 Aug 13, 2022
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 aio: preview area: testing Issues related to Angular testing features, such as TestBed merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants