Update tests in preparation for Angular 13 upgrade. #6056
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JamesHollyer
approved these changes
Nov 19, 2022
Contributor
JamesHollyer
left a comment
JamesHollyer
left a comment
There was a problem hiding this comment.
This seemed like a tedious change to make. Thanks for doing it!
| import {CategoryEnum} from '../_redux/notification_center_types'; | ||
| import {NotificationCenterComponent} from './notification_center_component'; | ||
| import {NotificationCenterContainer} from './notification_center_container'; | ||
| const testAction = createAction('[Notification] Notification Bell Clicked'); |
Contributor
There was a problem hiding this comment.
This is simple removed as cleanup?
Contributor
Author
There was a problem hiding this comment.
Yes, just a cleanup of an unused target.
bmd3k
added a commit
that referenced
this pull request
Nov 22, 2022
Upgrade Angular to version 13 and related dependencies appropriately. See: https://update.angular.io/?l=3&v=12.0-13.0 See: https://medium.com/ngrx/announcing-ngrx-version-13-ivy-builds-feature-creators-improved-selectors-and-more-6a1a4c52c824 Highlights: * Upgrade the following set of dependencies: * `@angular/*`: 13.3.X * `@ngrx/*`: 13.2.X * `rxjs`: 7.4.X * `zone.js` 0.11.6 or higher * `ngx-color-picker`: 12.0.1 * Some build tooling was previously upgraded in preparation for this change: * #6036 * #6049 * Some tests were previously cleaned up in preparation for this change: * #6056 * No longer need ngcc as a post install step. All Angular-related dependencies come with partially-compiled binaries. * The tensorboard:dev Angular binary now includes the runtime JIT compiler and no longer relies on AOT compilation. Googlers, see http://go/tb-oss-bundling for justification. * We can get rid of some of the patches to Angular build tooling that we needed while we used an old version of @angular/compiler-cli. * Impact on binaries and performance: * There isn't much impact on production binary sizes. The '//tensorboard' tb_webapp_binary.js shrinks slightly from 2,574,467 bytes to 2,559,890 bytes. * The '//tensorboard:dev' tb_webapp_binary.js grows an entire 1MB, from 6,074,186 bytes to 7,005,529, likely because it now includes the Angular compiler. It now relies on JIT compilation but there does not seem to be noticeable impact on runtime performance. The development experience is still relatively smooth.
qihach64
pushed a commit
to qihach64/tensorboard
that referenced
this pull request
Dec 19, 2022
The upcoming upgrade to Angular 13 (and related libraries) requires some modifications to tests that we can make pre-upgrade. 1. ngrx can no longer automatically reset selectors that have been modified with overrideSelector() call. We must now call store.resetSelectors() after each test. * See: ngrx/platform#3264 (comment) 2. The upgrade has exposed a latent test bug. In one of the tests there is the potential to refresh the window with a call to `window.location.refresh()`. After the Angular 13 upgrade the refresh actually happens and the test karma runner fails and complains "Some of your tests did a full page reload!". The fix is to mock out the call to `window.location.refresh()`.
qihach64
pushed a commit
to qihach64/tensorboard
that referenced
this pull request
Dec 19, 2022
) Upgrade Angular to version 13 and related dependencies appropriately. See: https://update.angular.io/?l=3&v=12.0-13.0 See: https://medium.com/ngrx/announcing-ngrx-version-13-ivy-builds-feature-creators-improved-selectors-and-more-6a1a4c52c824 Highlights: * Upgrade the following set of dependencies: * `@angular/*`: 13.3.X * `@ngrx/*`: 13.2.X * `rxjs`: 7.4.X * `zone.js` 0.11.6 or higher * `ngx-color-picker`: 12.0.1 * Some build tooling was previously upgraded in preparation for this change: * tensorflow#6036 * tensorflow#6049 * Some tests were previously cleaned up in preparation for this change: * tensorflow#6056 * No longer need ngcc as a post install step. All Angular-related dependencies come with partially-compiled binaries. * The tensorboard:dev Angular binary now includes the runtime JIT compiler and no longer relies on AOT compilation. Googlers, see http://go/tb-oss-bundling for justification. * We can get rid of some of the patches to Angular build tooling that we needed while we used an old version of @angular/compiler-cli. * Impact on binaries and performance: * There isn't much impact on production binary sizes. The '//tensorboard' tb_webapp_binary.js shrinks slightly from 2,574,467 bytes to 2,559,890 bytes. * The '//tensorboard:dev' tb_webapp_binary.js grows an entire 1MB, from 6,074,186 bytes to 7,005,529, likely because it now includes the Angular compiler. It now relies on JIT compilation but there does not seem to be noticeable impact on runtime performance. The development experience is still relatively smooth.
bmd3k
added a commit
that referenced
this pull request
Mar 24, 2023
Feature flag tests have been flaky since the Angular 13/14 upgrade. The problematic change was actually #6056, where we started using resetSelectors everywhere. There is one test where a dialog with FeatureFlagDialogContainer is opened. Sometimes FeatureFlagDialogContainer would not complete initialization until after the test was torn down and the selectors reset, meaning NgRx selector calls would fail since state is undefined at that point. To workaround this problem, I allow FeatureFlagDialogContainer to be replaced by a simpler TestableFeatureFlagDialogComponent that has no dependencies on NgRx. This is a strategy we've used elsewhere in the code: https://github.com/tensorflow/tensorboard/blob/dca18190a18ac151437bb6fcd128bc49af617ae5/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts#L206 While I was in this code, I did some other cleanup: * Stop running the feature flag view tests twice. Only run them as part of the feature flag test suite. * When FeatureFlagModalTriggerContainer is destroyed, unsubscribe from some subjects created in ngOnInit. * Remove some unnecessary setup from FeatureFlagModalTriggerContainer tests. Alternative solution: * I thought I could solve the problem by calling dialog.close() in the tests but observed that closing the dialog is not enough to prevent the child component from instantiating after the test.
yatbear
pushed a commit
to yatbear/tensorboard
that referenced
this pull request
Mar 27, 2023
The upcoming upgrade to Angular 13 (and related libraries) requires some modifications to tests that we can make pre-upgrade. 1. ngrx can no longer automatically reset selectors that have been modified with overrideSelector() call. We must now call store.resetSelectors() after each test. * See: ngrx/platform#3264 (comment) 2. The upgrade has exposed a latent test bug. In one of the tests there is the potential to refresh the window with a call to `window.location.refresh()`. After the Angular 13 upgrade the refresh actually happens and the test karma runner fails and complains "Some of your tests did a full page reload!". The fix is to mock out the call to `window.location.refresh()`.
yatbear
pushed a commit
to yatbear/tensorboard
that referenced
this pull request
Mar 27, 2023
) Upgrade Angular to version 13 and related dependencies appropriately. See: https://update.angular.io/?l=3&v=12.0-13.0 See: https://medium.com/ngrx/announcing-ngrx-version-13-ivy-builds-feature-creators-improved-selectors-and-more-6a1a4c52c824 Highlights: * Upgrade the following set of dependencies: * `@angular/*`: 13.3.X * `@ngrx/*`: 13.2.X * `rxjs`: 7.4.X * `zone.js` 0.11.6 or higher * `ngx-color-picker`: 12.0.1 * Some build tooling was previously upgraded in preparation for this change: * tensorflow#6036 * tensorflow#6049 * Some tests were previously cleaned up in preparation for this change: * tensorflow#6056 * No longer need ngcc as a post install step. All Angular-related dependencies come with partially-compiled binaries. * The tensorboard:dev Angular binary now includes the runtime JIT compiler and no longer relies on AOT compilation. Googlers, see http://go/tb-oss-bundling for justification. * We can get rid of some of the patches to Angular build tooling that we needed while we used an old version of @angular/compiler-cli. * Impact on binaries and performance: * There isn't much impact on production binary sizes. The '//tensorboard' tb_webapp_binary.js shrinks slightly from 2,574,467 bytes to 2,559,890 bytes. * The '//tensorboard:dev' tb_webapp_binary.js grows an entire 1MB, from 6,074,186 bytes to 7,005,529, likely because it now includes the Angular compiler. It now relies on JIT compilation but there does not seem to be noticeable impact on runtime performance. The development experience is still relatively smooth.
yatbear
pushed a commit
to yatbear/tensorboard
that referenced
this pull request
Mar 27, 2023
Feature flag tests have been flaky since the Angular 13/14 upgrade. The problematic change was actually tensorflow#6056, where we started using resetSelectors everywhere. There is one test where a dialog with FeatureFlagDialogContainer is opened. Sometimes FeatureFlagDialogContainer would not complete initialization until after the test was torn down and the selectors reset, meaning NgRx selector calls would fail since state is undefined at that point. To workaround this problem, I allow FeatureFlagDialogContainer to be replaced by a simpler TestableFeatureFlagDialogComponent that has no dependencies on NgRx. This is a strategy we've used elsewhere in the code: https://github.com/tensorflow/tensorboard/blob/dca18190a18ac151437bb6fcd128bc49af617ae5/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts#L206 While I was in this code, I did some other cleanup: * Stop running the feature flag view tests twice. Only run them as part of the feature flag test suite. * When FeatureFlagModalTriggerContainer is destroyed, unsubscribe from some subjects created in ngOnInit. * Remove some unnecessary setup from FeatureFlagModalTriggerContainer tests. Alternative solution: * I thought I could solve the problem by calling dialog.close() in the tests but observed that closing the dialog is not enough to prevent the child component from instantiating after the test.
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
The upcoming upgrade to Angular 13 (and related libraries) requires some modifications to tests that we can make pre-upgrade. 1. ngrx can no longer automatically reset selectors that have been modified with overrideSelector() call. We must now call store.resetSelectors() after each test. * See: ngrx/platform#3264 (comment) 2. The upgrade has exposed a latent test bug. In one of the tests there is the potential to refresh the window with a call to `window.location.refresh()`. After the Angular 13 upgrade the refresh actually happens and the test karma runner fails and complains "Some of your tests did a full page reload!". The fix is to mock out the call to `window.location.refresh()`.
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
) Upgrade Angular to version 13 and related dependencies appropriately. See: https://update.angular.io/?l=3&v=12.0-13.0 See: https://medium.com/ngrx/announcing-ngrx-version-13-ivy-builds-feature-creators-improved-selectors-and-more-6a1a4c52c824 Highlights: * Upgrade the following set of dependencies: * `@angular/*`: 13.3.X * `@ngrx/*`: 13.2.X * `rxjs`: 7.4.X * `zone.js` 0.11.6 or higher * `ngx-color-picker`: 12.0.1 * Some build tooling was previously upgraded in preparation for this change: * tensorflow#6036 * tensorflow#6049 * Some tests were previously cleaned up in preparation for this change: * tensorflow#6056 * No longer need ngcc as a post install step. All Angular-related dependencies come with partially-compiled binaries. * The tensorboard:dev Angular binary now includes the runtime JIT compiler and no longer relies on AOT compilation. Googlers, see http://go/tb-oss-bundling for justification. * We can get rid of some of the patches to Angular build tooling that we needed while we used an old version of @angular/compiler-cli. * Impact on binaries and performance: * There isn't much impact on production binary sizes. The '//tensorboard' tb_webapp_binary.js shrinks slightly from 2,574,467 bytes to 2,559,890 bytes. * The '//tensorboard:dev' tb_webapp_binary.js grows an entire 1MB, from 6,074,186 bytes to 7,005,529, likely because it now includes the Angular compiler. It now relies on JIT compilation but there does not seem to be noticeable impact on runtime performance. The development experience is still relatively smooth.
dna2github
pushed a commit
to dna2fork/tensorboard
that referenced
this pull request
May 1, 2023
Feature flag tests have been flaky since the Angular 13/14 upgrade. The problematic change was actually tensorflow#6056, where we started using resetSelectors everywhere. There is one test where a dialog with FeatureFlagDialogContainer is opened. Sometimes FeatureFlagDialogContainer would not complete initialization until after the test was torn down and the selectors reset, meaning NgRx selector calls would fail since state is undefined at that point. To workaround this problem, I allow FeatureFlagDialogContainer to be replaced by a simpler TestableFeatureFlagDialogComponent that has no dependencies on NgRx. This is a strategy we've used elsewhere in the code: https://github.com/tensorflow/tensorboard/blob/dca18190a18ac151437bb6fcd128bc49af617ae5/tensorboard/webapp/metrics/views/card_renderer/scalar_card_container.ts#L206 While I was in this code, I did some other cleanup: * Stop running the feature flag view tests twice. Only run them as part of the feature flag test suite. * When FeatureFlagModalTriggerContainer is destroyed, unsubscribe from some subjects created in ngOnInit. * Remove some unnecessary setup from FeatureFlagModalTriggerContainer tests. Alternative solution: * I thought I could solve the problem by calling dialog.close() in the tests but observed that closing the dialog is not enough to prevent the child component from instantiating after the test.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The upcoming upgrade to Angular 13 (and related libraries) requires some modifications to tests that we can make pre-upgrade.
window.location.refresh(). After the Angular 13 upgrade the refresh actually happens and the test karma runner fails and complains "Some of your tests did a full page reload!". The fix is to mock out the call towindow.location.refresh().