Skip to content

Commit

Permalink
Update tests in preparation for Angular 13 upgrade. (tensorflow#6056)
Browse files Browse the repository at this point in the history
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()`.
  • Loading branch information
bmd3k authored Nov 21, 2022
1 parent 888dcdc commit ed5e9d0
Show file tree
Hide file tree
Showing 65 changed files with 265 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,10 @@ describe('plugin_api_host test', () => {
runApi = TestBed.inject(PluginRunsApiHostImpl);
});

afterEach(() => {
store?.resetSelectors();
});

describe('runs apis', () => {
describe('#experimental.RunsChanged', () => {
it('broadcasts runs when runs change', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,10 @@ describe('Debugger effects', () => {
store.overrideSelector(getActivePlugin, '');
});

afterEach(() => {
store?.resetSelectors();
});

function createAndSubscribeToDebuggerEffectsWithEmptyRepeater() {
spyOn(TEST_ONLY, 'createTimedRepeater').and.callFake(
(stream: Observable<any>) => stream
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ describe('Graph Container', () => {
dispatchSpy = spyOn(store, 'dispatch');
});

afterEach(() => {
store?.resetSelectors();
});

it('renders no-op-selected element and nothing else when no op is selected', () => {
const fixture = TestBed.createComponent(GraphContainer);
store.overrideSelector(getFocusedGraphOpInfo, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ describe('Graph Executions Container', () => {
store = TestBed.inject<Store<State>>(Store) as MockStore<State>;
});

afterEach(() => {
store?.resetSelectors();
});

it('does not render execs viewport if # execs = 0', fakeAsync(() => {
const fixture = TestBed.createComponent(GraphExecutionsContainer);
store.overrideSelector(getNumGraphExecutions, 0);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ describe('Source Files Container', () => {

afterEach(() => {
tearDownMonacoFakes();
store?.resetSelectors();
});

it('renders no file selected when no source line is focused on', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,10 @@ describe('Stack Trace container', () => {
dispatchSpy = spyOn(store, 'dispatch');
});

afterEach(() => {
store?.resetSelectors();
});

for (const stickToBottommostFrame of [false, true]) {
it(
`shows non-empty eager stack frames; highlights focused frame: ` +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ describe('Timeline Container', () => {
dispatchSpy = spyOn(store, 'dispatch');
});

afterEach(() => {
store?.resetSelectors();
});

it('shows total number of executions', () => {
const fixture = TestBed.createComponent(TimelineContainer);
fixture.detectChanges();
Expand Down
1 change: 1 addition & 0 deletions tensorboard/webapp/alert/views/alert_snackbar_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ describe('alert snackbar', () => {

afterEach(() => {
snackbar.dismiss();
store?.resetSelectors();
});

it('opens the snackbar on each alert', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,10 @@ describe('app_routing_effects', () => {
);
});

afterEach(() => {
store?.resetSelectors();
});

describe('bootstrapReducers$', () => {
beforeEach(() => {
effects = TestBed.inject(AppRoutingEffects);
Expand Down
4 changes: 4 additions & 0 deletions tensorboard/webapp/app_routing/views/router_outlet_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ describe('router_outlet', () => {
getNgComponentByRouteKindSpy = spyOn(registry, 'getNgComponentByRouteKind');
});

afterEach(() => {
store?.resetSelectors();
});

function setActiveRoute(routeKind: RouteKind) {
store.overrideSelector(
getActiveRoute,
Expand Down
1 change: 1 addition & 0 deletions tensorboard/webapp/core/effects/core_effects_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ describe('core_effects', () => {

afterEach(() => {
httpMock.verify();
store?.resetSelectors();
});

[
Expand Down
1 change: 1 addition & 0 deletions tensorboard/webapp/core/views/dark_mode_supporter_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('dark mode supporter test', () => {

afterEach(() => {
document.body.classList.remove('dark-mode');
store?.resetSelectors();
});

it('sets class name on body when dark mode is enabled', () => {
Expand Down
4 changes: 4 additions & 0 deletions tensorboard/webapp/core/views/hash_storage_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,10 @@ describe('hash storage test', () => {
getPluginIdSpy = spyOn(deepLinker, 'getPluginId');
});

afterEach(() => {
store?.resetSelectors();
});

it('sets hash to plugin id when plugin id changes from null to value', () => {
const setPluginIdCalls: Array<{
id: string | null;
Expand Down
4 changes: 4 additions & 0 deletions tensorboard/webapp/core/views/layout_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ describe('layout test', () => {
store.overrideSelector(getSideBarWidthInPercent, 10);
});

afterEach(() => {
store?.resetSelectors();
});

it('renders sidebar and main content', () => {
const fixture = TestBed.createComponent(TestableComponent);
fixture.detectChanges();
Expand Down
12 changes: 12 additions & 0 deletions tensorboard/webapp/core/views/page_title_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ describe('page title test', () => {
});
});

afterEach(() => {
store?.resetSelectors();
});

it('uses window_title as page title if given', () => {
const spy = spyOn(TEST_ONLY.utils, 'setDocumentTitle');
store.overrideSelector(getRouteKind, RouteKind.EXPERIMENT);
Expand Down Expand Up @@ -139,6 +143,10 @@ describe('page title test with custom brand names', () => {
});
});

afterEach(() => {
store?.resetSelectors();
});

it('uses TensorBoard brand name as page title as default', () => {
const spy = spyOn(TEST_ONLY.utils, 'setDocumentTitle');
const fixture = TestBed.createComponent(TestingComponent);
Expand Down Expand Up @@ -213,6 +221,10 @@ describe('page title test for OSS TensorBoard', () => {
});
});

afterEach(() => {
store?.resetSelectors();
});

it('uses `TensorBoard` for experiment page title', () => {
const spy = spyOn(TEST_ONLY.utils, 'setDocumentTitle');
store.overrideSelector(getRouteKind, RouteKind.EXPERIMENT);
Expand Down
4 changes: 4 additions & 0 deletions tensorboard/webapp/deeplink/hash_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,10 @@ describe('hash storage test', () => {
createElement.and.callThrough();
});

afterEach(() => {
store?.resetSelectors();
});

it('sets the hash to plugin id by replacing on first load', () => {
store.overrideSelector(getActivePlugin, 'foo');
const fixture = TestBed.createComponent(HashStorageContainer);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ describe('feature_flag_effects', () => {
store.overrideSelector(getFeatureFlagsMetadata, FeatureFlagMetadataMap);
});

afterEach(() => {
store?.resetSelectors();
});

describe('getFeatureFlags$', () => {
let recordedActions: Action[];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ describe('FeatureFlagHttpInterceptor', () => {
httpClient = TestBed.inject(HttpClient);
});

afterEach(() => {
store?.resetSelectors();
});

it('injects feature flags into the HTTP request', () => {
store.overrideSelector(getFeatureFlagsToSendToServer, {
inColab: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ import {featureFlagOverridesReset} from '../actions/feature_flag_actions';
import {getShowFlagsEnabled} from '../store/feature_flag_selectors';
import {FeatureFlagPageContainer} from './feature_flag_page_container';

const util = {
reloadWindow: () => {
window.location.reload();
},
};

@Component({
selector: 'feature-flag-modal-trigger',
template: ``,
Expand Down Expand Up @@ -51,11 +57,15 @@ export class FeatureFlagModalTriggerContainer implements OnInit {
// Wait one tick before reloading the page so the 'enableShowFlags'
// reset has a chance to be reflected in the URL before page reload.
setTimeout(() => {
window.location.reload();
util.reloadWindow();
}, 1);
});
return;
}
});
}
}

export const TEST_ONLY = {
util,
};
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,10 @@ import {
getShowFlagsEnabled,
} from '../store/feature_flag_selectors';
import {FeatureFlags} from '../types';
import {FeatureFlagModalTriggerContainer} from './feature_flag_modal_trigger_container';
import {
FeatureFlagModalTriggerContainer,
TEST_ONLY,
} from './feature_flag_modal_trigger_container';
import {FeatureFlagPageContainer} from './feature_flag_page_container';

describe('feature_flag_modal_trigger_container', () => {
Expand All @@ -58,6 +61,13 @@ describe('feature_flag_modal_trigger_container', () => {
store = TestBed.inject<Store<State>>(Store) as MockStore<State>;

spyOn(store, 'dispatch').and.stub();

// Fake out window.location.reload so it doesn't do anything.
TEST_ONLY.util.reloadWindow = () => {};
});

afterEach(() => {
store?.resetSelectors();
});

function createComponent() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ describe('feature_flag_page_container', () => {
dispatchSpy = spyOn(store, 'dispatch');
});

afterEach(() => {
store?.resetSelectors();
});

function createComponent() {
fixture = TestBed.createComponent(FeatureFlagPageContainer);
fixture.detectChanges();
Expand Down
4 changes: 4 additions & 0 deletions tensorboard/webapp/header/dark_mode_toggle_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ describe('dark mode toggle test', () => {
overlayContainer = TestBed.inject(OverlayContainer);
});

afterEach(() => {
store?.resetSelectors();
});

function getMenuButtons(fixture: ComponentFixture<DarkModeToggleContainer>) {
const els = fixture.debugElement.queryAll(By.css('button'));
expect(els.length).toBe(1);
Expand Down
4 changes: 4 additions & 0 deletions tensorboard/webapp/header/header_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ describe('header test', () => {
store.overrideSelector(getCoreDataLoadedState, DataLoadState.NOT_LOADED);
});

afterEach(() => {
store?.resetSelectors();
});

function assertDebugElementText(el: DebugElement, text: string) {
expect(el.nativeElement.innerText.trim().toUpperCase()).toBe(text);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ describe('TBMetricsDataSource test', () => {

afterEach(() => {
httpMock.verify();
store?.resetSelectors();
});

describe('fetchTagMetadata', () => {
Expand Down
4 changes: 4 additions & 0 deletions tensorboard/webapp/metrics/effects/metrics_effects_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,10 @@ describe('metrics effects', () => {
);
});

afterEach(() => {
store?.resetSelectors();
});

describe('#dataEffects', () => {
beforeEach(() => {
effects.dataEffects$.subscribe();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ describe('card view test', () => {
intersectionObserver = TestBed.inject(IntersectionObserverTestingModule);
});

afterEach(() => {
store?.resetSelectors();
});

it('stamps DOM only when it is first visible', () => {
const fixture = TestBed.createComponent(CardViewContainer);
fixture.componentInstance.cardId = 'cardId';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,10 @@ describe('metrics/views/data_download_dialog', () => {
return fixture;
}

afterEach(() => {
store?.resetSelectors();
});

it('renders', async () => {
const fixture = await createComponent('card1');
store.overrideSelector(getCardMetadata, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ describe('histogram card', () => {
store.overrideSelector(selectors.getMetricsLinkedTimeSelection, null);
});

afterEach(() => {
store?.resetSelectors();
});

it('renders empty message when there is no data', () => {
const cardMetadata = {
plugin: PluginType.HISTOGRAMS,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ describe('image card', () => {
store.overrideSelector(getRun, null);
});

afterEach(() => {
store?.resetSelectors();
});

function expectImageSliderUI(
fixture: ComponentFixture<ImageCardContainer>,
imageId: string,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@ describe('card run name', () => {
store.overrideSelector(getRun, null);
});

afterEach(() => {
store?.resetSelectors();
});

it('renders exp display name and run name', () => {
store.overrideSelector(getExperimentIdForRunId, 'eid');
store.overrideSelector(getExperimentIdToExperimentAliasMap, {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,10 @@ describe('scalar card', () => {
);
});

afterEach(() => {
store?.resetSelectors();
});

describe('basic renders', () => {
it('renders empty chart when there is no data', fakeAsync(() => {
const cardMetadata = {
Expand Down
4 changes: 4 additions & 0 deletions tensorboard/webapp/metrics/views/main_view/card_grid_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,10 @@ describe('card grid', () => {
store.overrideSelector(getMetricsXAxisType, XAxisType.STEP);
});

afterEach(() => {
store?.resetSelectors();
});

it('keeps pagination button position when page size changes', fakeAsync(() => {
store.overrideSelector(settingsSelectors.getPageSize, 2);
let scrollOffset = 30;
Expand Down
Loading

0 comments on commit ed5e9d0

Please sign in to comment.