Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
28 changes: 25 additions & 3 deletions tensorboard/webapp/reloader/reloader_component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
import {Component, ChangeDetectionStrategy} from '@angular/core';
import {DOCUMENT} from '@angular/common';
import {Component, ChangeDetectionStrategy, Inject} from '@angular/core';
import {Store, select} from '@ngrx/store';
import {combineLatest} from 'rxjs';
import {distinctUntilChanged} from 'rxjs/operators';
Expand All @@ -28,15 +29,21 @@ import {reload} from '../core/actions';
changeDetection: ChangeDetectionStrategy.OnPush,
})
export class ReloaderComponent {
private readonly onVisibilityChange = this.onVisibilityChangeImpl.bind(this);
private readonly reloadEnabled$ = this.store.pipe(select(getReloadEnabled));
private readonly reloadPeriodInMs$ = this.store.pipe(
select(getReloadPeriodInMs)
);
private reloadTimerId: ReturnType<typeof setTimeout> | null = null;
private missedAutoReload: boolean = false;

constructor(private store: Store<State>) {}
constructor(
private store: Store<State>,
@Inject(DOCUMENT) private readonly document: Document
) {}

ngOnInit() {
this.document.addEventListener('visibilitychange', this.onVisibilityChange);
combineLatest(
this.reloadEnabled$.pipe(distinctUntilChanged()),
this.reloadPeriodInMs$.pipe(distinctUntilChanged())
Expand All @@ -48,9 +55,20 @@ export class ReloaderComponent {
});
}

private onVisibilityChangeImpl() {
if (this.document.visibilityState === 'visible' && this.missedAutoReload) {
this.missedAutoReload = false;
this.store.dispatch(reload());
}
}

private load(reloadPeriodInMs: number) {
this.reloadTimerId = setTimeout(() => {
this.store.dispatch(reload());
if (this.document.visibilityState === 'visible') {
this.store.dispatch(reload());
} else {
this.missedAutoReload = true;
}
this.load(reloadPeriodInMs);
}, reloadPeriodInMs);
}
Expand All @@ -64,5 +82,9 @@ export class ReloaderComponent {

ngOnDestroy() {
this.cancelLoad();
this.document.removeEventListener(
'visibilitychange',
this.onVisibilityChange
);
}
}
183 changes: 182 additions & 1 deletion tensorboard/webapp/reloader/reloader_component_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
==============================================================================*/
import {DOCUMENT} from '@angular/common';
import {TestBed, fakeAsync, tick} from '@angular/core/testing';
import {Store} from '@ngrx/store';
import {provideMockStore, MockStore} from '@ngrx/store/testing';
Expand All @@ -27,10 +28,34 @@ import {createState, createCoreState} from '../core/testing';
describe('reloader_component', () => {
let store: MockStore<State>;
let dispatchSpy: jasmine.Spy;
let fakeDocument: Document;

function createFakeDocument() {
return {
visibilityState: 'visible',
addEventListener: document.addEventListener.bind(document),
removeEventListener: document.removeEventListener.bind(document),
// DOMTestComponentRenderer injects DOCUMENT and requires the following
// properties to function.
querySelectorAll: document.querySelectorAll.bind(document),
body: document.body,
};
}

function simulateVisibilityChange(visible: boolean) {
Object.defineProperty(fakeDocument, 'visibilityState', {
get: () => (visible ? 'visible' : 'hidden'),
});
document.dispatchEvent(new Event('visibilitychange'));
}

beforeEach(async () => {
await TestBed.configureTestingModule({
providers: [
{
provide: DOCUMENT,
useFactory: createFakeDocument,
},
provideMockStore({
initialState: createState(
createCoreState({
Expand All @@ -44,6 +69,7 @@ describe('reloader_component', () => {
declarations: [ReloaderComponent],
}).compileComponents();
store = TestBed.get(Store);
fakeDocument = TestBed.get(DOCUMENT);
Copy link
Contributor

Choose a reason for hiding this comment

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

just wondering, what happens if you change this to TestBed.get(DOCUMENT) as typeof document?
I want to avoid any where possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

typeof document doesn't seem to help. I can (and have) made fakeDocument: Document but it requires some slight grossness to override the visibilityState property. That's ok - I think the grossness is outweighed by the better typing.

Copy link
Contributor

Choose a reason for hiding this comment

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

Acknowledged.

dispatchSpy = spyOn(store, 'dispatch');
});

Expand All @@ -69,7 +95,8 @@ describe('reloader_component', () => {
expect(dispatchSpy).toHaveBeenCalledTimes(2);
expect(dispatchSpy).toHaveBeenCalledWith(reload());

// // Manually invoke destruction of the component so we can cleanup the timer.
// Manually invoke destruction of the component so we can cleanup the
// timer.
fixture.destroy();
}));

Expand Down Expand Up @@ -161,4 +188,158 @@ describe('reloader_component', () => {

fixture.destroy();
}));

it('does not reload if document is not visible', fakeAsync(() => {
store.setState(
createState(
createCoreState({
reloadPeriodInMs: 5,
reloadEnabled: true,
})
)
);
const fixture = TestBed.createComponent(ReloaderComponent);
fixture.detectChanges();

tick(5);
expect(dispatchSpy).toHaveBeenCalledTimes(1);

simulateVisibilityChange(false);
tick(5);
expect(dispatchSpy).toHaveBeenCalledTimes(1);

fixture.destroy();
}));

it('reloads when document becomes visible if missed reload', fakeAsync(() => {
store.setState(
createState(
createCoreState({
reloadPeriodInMs: 5,
reloadEnabled: true,
})
)
);
const fixture = TestBed.createComponent(ReloaderComponent);
fixture.detectChanges();

// Miss a reload because not visible.
simulateVisibilityChange(false);
tick(5);
expect(dispatchSpy).toHaveBeenCalledTimes(0);

// Dispatch a reload when next visible.
simulateVisibilityChange(true);
expect(dispatchSpy).toHaveBeenCalledTimes(1);

fixture.destroy();
}));

it('reloads when document becomes visible if missed reload, regardless of how long not visible', fakeAsync(() => {
store.setState(
createState(
createCoreState({
reloadPeriodInMs: 5,
reloadEnabled: true,
})
)
);
const fixture = TestBed.createComponent(ReloaderComponent);
fixture.detectChanges();

tick(5);
expect(dispatchSpy).toHaveBeenCalledTimes(1);

// Document is not visible during time period that includes missed auto
// reload but is less than reloadPeriodInMs.
tick(2);
simulateVisibilityChange(false);
tick(3);
// No reload is dispatched.
expect(dispatchSpy).toHaveBeenCalledTimes(1);

// Dispatch a reload when next visible.
simulateVisibilityChange(true);
expect(dispatchSpy).toHaveBeenCalledTimes(2);

fixture.destroy();
}));

it('does not reload when document becomes visible if there was not a missed reload', fakeAsync(() => {
store.setState(
createState(
createCoreState({
reloadPeriodInMs: 5,
reloadEnabled: true,
})
)
);
const fixture = TestBed.createComponent(ReloaderComponent);
fixture.detectChanges();

tick(5);
expect(dispatchSpy).toHaveBeenCalledTimes(1);

// Document is not visible during time period that does not include
// missed auto reload.
simulateVisibilityChange(false);
tick(3);
simulateVisibilityChange(true);
expect(dispatchSpy).toHaveBeenCalledTimes(1);

fixture.destroy();
}));

it('does not reload when document becomes visible if missed reload was already handled', fakeAsync(() => {
store.setState(
createState(
createCoreState({
reloadPeriodInMs: 5,
reloadEnabled: true,
})
)
);
const fixture = TestBed.createComponent(ReloaderComponent);
fixture.detectChanges();

// Miss a reload because not visible.
simulateVisibilityChange(false);
tick(6);
expect(dispatchSpy).toHaveBeenCalledTimes(0);

// Dispatch a reload when next visible.
simulateVisibilityChange(true);
expect(dispatchSpy).toHaveBeenCalledTimes(1);

// Document is not visible during time period that does not include
// another missed reload.
simulateVisibilityChange(false);
tick(2);
simulateVisibilityChange(true);
// No additional reload dispatched.
expect(dispatchSpy).toHaveBeenCalledTimes(1);

fixture.destroy();
}));

it('does not reload when document becomes visible if auto reload is off', fakeAsync(() => {
store.setState(
createState(
createCoreState({
reloadPeriodInMs: 5,
reloadEnabled: false,
})
)
);
const fixture = TestBed.createComponent(ReloaderComponent);
fixture.detectChanges();

simulateVisibilityChange(false);
tick(5);

simulateVisibilityChange(true);
expect(dispatchSpy).toHaveBeenCalledTimes(0);

fixture.destroy();
}));
});