Skip to content

ng: reload now only reloads current dashboard #3426

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

Merged
stephanwlee merged 3 commits into from
Mar 25, 2020
Merged

ng: reload now only reloads current dashboard #3426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a39705c
ng: reload now only reloads current dashboard
stephanwlee Mar 24, 2020
956aa1e
cr address
stephanwlee Mar 24, 2020
889f9e1
CI kick
stephanwlee Mar 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions tensorboard/webapp/plugins/plugins_component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ export class PluginsComponent implements OnChanges {
private readonly ngPluginContainer!: ViewContainerRef;

@Input()
activePlugin?: UiPluginMetadata;
activePlugin!: UiPluginMetadata | null;

@Input()
lastUpdated?: number;
Expand Down Expand Up @@ -140,11 +140,15 @@ export class PluginsComponent implements OnChanges {
}

private reload() {
for (const instance of this.pluginInstances.values()) {
const maybePolymerDashboard = instance as any;
if (maybePolymerDashboard.reload) {
maybePolymerDashboard.reload();
}
if (!this.activePlugin) {
return;
}

const maybeDashboard = this.pluginInstances.get(
this.activePlugin.id
) as any;
if (maybeDashboard.reload) {
maybeDashboard.reload();
}
}
}
170 changes: 90 additions & 80 deletions tensorboard/webapp/plugins/plugins_container_test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,26 @@ import {provideMockStore, MockStore} from '@ngrx/store/testing';

import {PluginsContainer} from './plugins_container';
import {PluginsComponent} from './plugins_component';

import {PluginId, LoadingMechanismType} from '../types/api';
import {PluginRegistryModule} from './plugin_registry_module';
import {ExtraDashboardModule} from './testing';

import {
PluginId,
LoadingMechanismType,
CustomElementLoadingMechanism,
IframeLoadingMechanism,
NgElementLoadingMechanism,
} from '../types/api';
import {DataLoadState} from '../types/data';
import {createState, createCoreState} from '../core/testing';
import {State} from '../core/store';
// store/index.ts doesn't export this, but it's OK to use for testing
import {CoreState} from '../core/store/core_types';

import {
getPlugins,
getActivePlugin,
getPluginsListLoaded,
} from '../core/store/core_selectors';
import {TestingDebuggerModule} from '../../plugins/debugger_v2/tf_debugger_v2_plugin/testing';

/** @typehack */ import * as _typeHackStore from '@ngrx/store';
import {PluginRegistryModule} from './plugin_registry_module';
import {ExtraDashboardComponent, ExtraDashboardModule} from './testing';

function expectPluginIframe(element: HTMLElement, name: string) {
expect(element.tagName).toBe('IFRAME');
Expand All @@ -42,72 +49,61 @@ function expectPluginIframe(element: HTMLElement, name: string) {

describe('plugins_component', () => {
let store: MockStore<State>;
const INITIAL_CORE_STATE: Partial<CoreState> = {
plugins: {
bar: {
disable_reload: false,
enabled: true,
loading_mechanism: {
type: LoadingMechanismType.CUSTOM_ELEMENT,
element_name: 'tb-bar',
},
tab_name: 'Bar',
remove_dom: false,
},
'extra-plugin': {
disable_reload: false,
enabled: true,
loading_mechanism: {
type: LoadingMechanismType.NG_COMPONENT,
},
tab_name: 'Extra',
remove_dom: false,
},
foo: {
disable_reload: false,
enabled: true,
loading_mechanism: {
type: LoadingMechanismType.IFRAME,
// This will cause 404 as test bundles do not serve
// data file in the karma server.
module_path: 'random_esmodule.js',
},
tab_name: 'Bar',
remove_dom: false,
},
const PLUGINS = {
bar: {
disable_reload: false,
enabled: true,
loading_mechanism: {
type: LoadingMechanismType.CUSTOM_ELEMENT,
element_name: 'tb-bar',
} as CustomElementLoadingMechanism,
tab_name: 'Bar',
remove_dom: false,
},
'extra-plugin': {
disable_reload: false,
enabled: true,
loading_mechanism: {
type: LoadingMechanismType.NG_COMPONENT,
} as NgElementLoadingMechanism,
tab_name: 'Extra',
remove_dom: false,
},
foo: {
disable_reload: false,
enabled: true,
loading_mechanism: {
type: LoadingMechanismType.IFRAME,
// This will cause 404 as test bundles do not serve
// data file in the karma server.
module_path: 'random_esmodule.js',
} as IframeLoadingMechanism,
tab_name: 'Bar',
remove_dom: false,
},
};

function setActivePlugin(plugin: PluginId) {
store.overrideSelector(getActivePlugin, plugin);
store.refreshState();
}

beforeEach(async () => {
const initialState = createState(
createCoreState({
...INITIAL_CORE_STATE,
})
);
await TestBed.configureTestingModule({
providers: [
provideMockStore({initialState}),
PluginsContainer,
PluginRegistryModule,
],
providers: [provideMockStore(), PluginsContainer, PluginRegistryModule],
declarations: [PluginsContainer, PluginsComponent],
imports: [TestingDebuggerModule, ExtraDashboardModule],
}).compileComponents();
store = TestBed.get(Store);
store.overrideSelector(getPlugins, PLUGINS);
store.overrideSelector(getActivePlugin, null);
store.overrideSelector(getPluginsListLoaded, {
state: DataLoadState.NOT_LOADED,
lastLoadedTimeInMs: null,
});
});

describe('plugin DOM creation', () => {
function setActivePlugin(plugin: PluginId) {
store.setState(
createState(
createCoreState({
...INITIAL_CORE_STATE,
activePlugin: plugin,
})
)
);
}

it('creates no plugin when there is no activePlugin', () => {
const fixture = TestBed.createComponent(PluginsContainer);
const el = fixture.debugElement.query(By.css('.plugins'));
Expand Down Expand Up @@ -219,42 +215,56 @@ describe('plugins_component', () => {
timeInMs: number | null,
state = DataLoadState.LOADED
) {
store.setState(
createState(
createCoreState({
...INITIAL_CORE_STATE,
activePlugin: 'bar',
pluginsListLoaded: {
state,
lastLoadedTimeInMs: timeInMs,
},
})
)
);
store.overrideSelector(getPluginsListLoaded, {
state:
timeInMs !== null ? DataLoadState.LOADED : DataLoadState.NOT_LOADED,
lastLoadedTimeInMs: timeInMs,
});
store.refreshState();
}

it('invokes reload method on the dashboard DOM', () => {
const fixture = TestBed.createComponent(PluginsContainer);

setLastLoadedTime(null, DataLoadState.NOT_LOADED);
setActivePlugin('bar');
fixture.detectChanges();
setActivePlugin('foo');
fixture.detectChanges();
setActivePlugin('bar');
fixture.detectChanges();

const {nativeElement} = fixture.debugElement.query(By.css('.plugins'));
const [barElement] = nativeElement.children;
const reloadSpy = jasmine.createSpy();
barElement.reload = reloadSpy;
// Stamped 'bar' and 'foo'
expect(nativeElement.children.length).toBe(2);
const [barElement, fooElement] = nativeElement.children;
const barReloadSpy = jasmine.createSpy();
barElement.reload = barReloadSpy;
const fooReloadSpy = jasmine.createSpy();
fooElement.reload = fooReloadSpy;

setLastLoadedTime(1);
fixture.detectChanges();
expect(reloadSpy).toHaveBeenCalledTimes(1);
expect(barReloadSpy).toHaveBeenCalledTimes(1);
expect(fooReloadSpy).not.toHaveBeenCalled();

setLastLoadedTime(1);
fixture.detectChanges();
expect(reloadSpy).toHaveBeenCalledTimes(1);
expect(barReloadSpy).toHaveBeenCalledTimes(1);
expect(fooReloadSpy).not.toHaveBeenCalled();

setLastLoadedTime(2);
fixture.detectChanges();
expect(reloadSpy).toHaveBeenCalledTimes(2);
expect(barReloadSpy).toHaveBeenCalledTimes(2);
expect(fooReloadSpy).not.toHaveBeenCalled();

setActivePlugin('foo');
fixture.detectChanges();

setLastLoadedTime(3);
fixture.detectChanges();
expect(barReloadSpy).toHaveBeenCalledTimes(2);
expect(fooReloadSpy).toHaveBeenCalledTimes(1);
});
});
});