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.

Sign up for GitHub

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

Jump to bottom

perf(platform-browser): avoid including Testability by default in bootstrapApplication #45885

Closed
wants to merge 1 commit into from
Closed

perf(platform-browser): avoid including Testability by default in bootstrapApplication #45885

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions goldens/public-api/platform-browser/index.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,9 @@ export type MetaDefinition = {
// @public
export const platformBrowser: (extraProviders?: StaticProvider[]) => PlatformRef;

// @public
export function provideProtractorTestingSupport(): Provider[];

// @public
export interface SafeHtml extends SafeValue {
}
Expand Down
2 changes: 1 addition & 1 deletion goldens/size-tracking/integration-payloads.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@
"standalone-bootstrap": {
"uncompressed": {
"runtime": 1090,
"main": 87940,
"main": 84237,
"polyfills": 33945
}
},
Expand Down
31 changes: 9 additions & 22 deletions integration/standalone-bootstrap/e2e/src/app.e2e-spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,11 @@
import {browser, logging} from 'protractor';

import {AppPage} from './app.po';

/**
* Note: this file contains no e2e tests, since they rely on Protractor,
* which requires Testability. Testability is not included by default when
* the `bootstrapApplication` function is used (which is the case in this app).
* We use this app primarily to measure payload size, so we want to keep
* Testability excluded.
Copy link
Member

Choose a reason for hiding this comment

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

should we have this e2e test / e2e testing infrastructure at all then? Or even this integration/standalone-bootstrap integration test at all? It seems like we are trying to have 2 things covered by this "app":

  • size tracking;
  • actual integration testing of the bootstrap operation.

It seems like those 2 needs are in conflict somehow?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pkozlowski-opensource currently those two parts (size tracking and e2e tests) are bundled together in one build rule:

angular/integration/standalone-bootstrap/BUILD.bazel

Lines 3 to 7 in 74321a4

ng_integration_test(
name = "test",
setup_chromium = True,
track_payload_size = "standalone-bootstrap",
)

Ideally we'd need to decouple them into 2 separate build rules, so that we have more flexibility in terms of configuration. Once it's done, we can refactor the usage here to just leverage the size-tracking piece.

*/
describe('Standalone Bootstrap app', () => {
let page: AppPage;

beforeEach(() => {
page = new AppPage();
});

it('should display title', () => {
page.navigateTo();
expect(page.getTitleText()).toEqual('Standalone Bootstrap app');
});

afterEach(async () => {
// Assert that there are no errors emitted from the browser
const logs = await browser.manage().logs().get(logging.Type.BROWSER);
expect(logs).not.toContain(jasmine.objectContaining({
level: logging.Level.SEVERE,
} as logging.Entry));
});
// Jasmine will throw if there are no tests.
it('should pass', () => {});
});
18 changes: 16 additions & 2 deletions packages/core/src/testability/testability.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,22 @@ export const TESTABILITY_GETTER = new InjectionToken<GetTestability>('');

/**
* The Testability service provides testing hooks that can be accessed from
* the browser. Each bootstrapped Angular application on the page will have
* an instance of Testability.
* the browser.
*
* Angular applications bootstrapped using an NgModule (via `@NgModule.bootstrap` field) will also
* instantiate Testability by default (in both development and production modes).
*
* For applications bootstrapped using the `bootstrapApplication` function, Testability is not
* included by default. You can include it into your applications by getting the list of necessary
* providers using the `provideProtractorTestingSupport()` function and adding them into the
* `options.providers` array. Example:
*
* ```typescript
* import {provideProtractorTestingSupport} from '@angular/platform-browser';
*
* await bootstrapApplication(RootComponent, providers: [provideProtractorTestingSupport()]);
* ```
*
* @publicApi
*/
@Injectable()
Expand Down
24 changes: 0 additions & 24 deletions packages/core/test/bundling/standalone_bootstrap/bundle.golden_symbols.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,6 @@
{
"name": "DefaultDomRenderer2"
},
{
"name": "DomEventsPlugin"
},
{
"name": "DomRendererFactory2"
},
Expand Down Expand Up @@ -137,9 +134,6 @@
{
"name": "Injector"
},
{
"name": "KeyEventsPlugin"
},
{
"name": "LOCALE_ID2"
},
Expand Down Expand Up @@ -296,24 +290,12 @@
{
"name": "TESTABILITY"
},
{
"name": "TESTABILITY_GETTER"
},
{
"name": "TESTABILITY_PROVIDERS"
},
{
"name": "THROW_IF_NOT_FOUND"
},
{
"name": "TRACKED_LVIEWS"
},
{
"name": "Testability"
},
{
"name": "TestabilityRegistry"
},
{
"name": "USE_VALUE"
},
Expand Down Expand Up @@ -362,9 +344,6 @@
{
"name": "_renderCompCount"
},
{
"name": "_testabilityGetter"
},
{
"name": "_wrapInTimeout"
},
Expand Down Expand Up @@ -815,9 +794,6 @@
{
"name": "scheduleArray"
},
{
"name": "scheduleMicroTask"
},
{
"name": "searchTokensOnInjector"
},
Expand Down
33 changes: 30 additions & 3 deletions packages/platform-browser/src/browser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,17 @@ export interface ApplicationConfig {
* const appRef: ApplicationRef = await bootstrapApplication(RootComponent);
* ```
*
* Note: this bootstrap method doesn't include [Testability](api/core/Testability) by default.
* You can add [Testability](api/core/Testability) by getting the list of necessary providers
* using `provideProtractorTestingSupport()` function and add them into the `options.providers`
* array. Example:
*
* ```typescript
* import {provideProtractorTestingSupport} from '@angular/platform-browser';
*
* await bootstrapApplication(RootComponent, providers: [provideProtractorTestingSupport()]);
* ```
*
* @param rootComponent A reference to a Standalone Component that should be rendered.
* @param options Additional configuration for the bootstrap operation, see `ApplicationConfig` for
* additional info.
Expand All @@ -62,13 +73,29 @@ export function bootstrapApplication(
rootComponent,
appProviders: [
...BROWSER_MODULE_PROVIDERS,
...TESTABILITY_PROVIDERS,
...(options?.providers ?? []),
],
platformProviders: INTERNAL_BROWSER_PLATFORM_PROVIDERS,
});
}

/**
* Returns a set of providers required to setup [Testability](api/core/Testability) for an
* application bootstrapped using the `bootstrapApplication` function. The set of providers is
* needed to support testing an application with Protractor (which relies on the Testability APIs
* to be present).
*
* @returns An array of providers required to setup Testability for an application and make it
* available for testing using Protractor.
*
* @publicApi
*/
export function provideProtractorTestingSupport(): Provider[] {
// Return a copy to prevent changes to the original array in case any in-place
// alterations are performed to the `provideProtractorTestingSupport` call results in app code.
return [...TESTABILITY_PROVIDERS];
}

export function initDomAdapter() {
BrowserDomAdapter.makeCurrent();
}
Expand Down Expand Up @@ -107,7 +134,7 @@ export const platformBrowser: (extraProviders?: StaticProvider[]) => PlatformRef
const BROWSER_MODULE_PROVIDERS_MARKER =
new InjectionToken(NG_DEV_MODE ? 'BrowserModule Providers Marker' : '');

export const TESTABILITY_PROVIDERS = [
const TESTABILITY_PROVIDERS = [
{
provide: TESTABILITY_GETTER,
useClass: BrowserGetTestability,
Expand All @@ -125,7 +152,7 @@ export const TESTABILITY_PROVIDERS = [
}
];

export const BROWSER_MODULE_PROVIDERS: StaticProvider[] = [
const BROWSER_MODULE_PROVIDERS: Provider[] = [
{provide: INJECTOR_SCOPE, useValue: 'root'},
{provide: ErrorHandler, useFactory: errorHandler, deps: []}, {
provide: EVENT_MANAGER_PLUGINS,
Expand Down
2 changes: 1 addition & 1 deletion packages/platform-browser/src/platform-browser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

export {ApplicationConfig, bootstrapApplication, BrowserModule, platformBrowser} from './browser';
export {ApplicationConfig, bootstrapApplication, BrowserModule, platformBrowser, provideProtractorTestingSupport} from './browser';
export {Meta, MetaDefinition} from './browser/meta';
export {Title} from './browser/title';
export {disableDebugTools, enableDebugTools} from './browser/tools/tools';
Expand Down