From 32dfc74aff75827d7a94774b4fab7f3bf9a71034 Mon Sep 17 00:00:00 2001 From: Jari Kolehmainen Date: Tue, 23 Nov 2021 19:41:16 +0200 Subject: [PATCH] Add unit tests for BaseStore (#4404) * fix possible race condition in BaseStore persistence Signed-off-by: Jari Kolehmainen * cleanup variable names in tests Signed-off-by: Jari Kolehmainen * remove async Signed-off-by: Jari Kolehmainen --- src/common/__tests__/base-store.test.ts | 171 ++++++++++++++++++++++++ src/common/base-store.ts | 4 +- 2 files changed, 173 insertions(+), 2 deletions(-) create mode 100644 src/common/__tests__/base-store.test.ts diff --git a/src/common/__tests__/base-store.test.ts b/src/common/__tests__/base-store.test.ts new file mode 100644 index 0000000000..0a4f81e8f3 --- /dev/null +++ b/src/common/__tests__/base-store.test.ts @@ -0,0 +1,171 @@ +/** + * Copyright (c) 2021 OpenLens Authors + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to + * use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of + * the Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER + * IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ + +import mockFs from "mock-fs"; +import { AppPaths } from "../app-paths"; +import { BaseStore } from "../base-store"; +import { action, comparer, makeObservable, observable, toJS } from "mobx"; +import { readFileSync } from "fs"; + +AppPaths.init(); + +jest.mock("electron", () => ({ + app: { + getVersion: () => "99.99.99", + getName: () => "lens", + setName: jest.fn(), + setPath: jest.fn(), + getPath: () => "tmp", + getLocale: () => "en", + setLoginItemSettings: jest.fn(), + }, + ipcMain: { + handle: jest.fn(), + on: jest.fn(), + removeAllListeners: jest.fn(), + off: jest.fn(), + send: jest.fn(), + }, +})); + +interface TestStoreModel { + a: string; + b: string; + c: string; +} + +class TestStore extends BaseStore { + @observable a: string; + @observable b: string; + @observable c: string; + + constructor() { + super({ + configName: "test-store", + accessPropertiesByDotNotation: false, // To make dots safe in cluster context names + syncOptions: { + equals: comparer.structural, + }, + }); + + makeObservable(this); + this.load(); + } + + @action updateAll(data: TestStoreModel) { + this.a = data.a; + this.b = data.b; + this.c = data.c; + } + + @action fromStore(data: Partial = {}) { + this.a = data.a || ""; + this.b = data.b || ""; + this.c = data.c || ""; + } + + onSync(data: TestStoreModel) { + super.onSync(data); + } + + async saveToFile(model: TestStoreModel) { + return super.saveToFile(model); + } + + toJSON(): TestStoreModel { + const data: TestStoreModel = { + a: this.a, + b: this.b, + c: this.c, + }; + + return toJS(data); + } +} + +describe("BaseStore", () => { + let store: TestStore; + + beforeEach(async () => { + store = undefined; + TestStore.resetInstance(); + const mockOpts = { + "tmp": { + "test-store.json": JSON.stringify({}), + }, + }; + + mockFs(mockOpts); + + store = TestStore.createInstance(); + }); + + afterEach(() => { + store.disableSync(); + TestStore.resetInstance(); + mockFs.restore(); + }); + + describe("persistence", () => { + it("persists changes to the filesystem", () => { + store.updateAll({ + a: "foo", b: "bar", c: "hello", + }); + + const data = JSON.parse(readFileSync("tmp/test-store.json").toString()); + + expect(data).toEqual({ a: "foo", b: "bar", c: "hello" }); + }); + + it("persists transaction only once", () => { + const fileSpy = jest.spyOn(store, "saveToFile"); + + store.updateAll({ + a: "foo", b: "bar", c: "hello", + }); + + expect(fileSpy).toHaveBeenCalledTimes(1); + }); + + it("persists changes one-by-one without transaction", () => { + const fileSpy = jest.spyOn(store, "saveToFile"); + + store.a = "a"; + store.b = "b"; + + expect(fileSpy).toHaveBeenCalledTimes(2); + + const data = JSON.parse(readFileSync("tmp/test-store.json").toString()); + + expect(data).toEqual({ a: "a", b: "b", c: "" }); + }); + + it("persists changes coming via onSync (sync from different process)", () => { + const fileSpy = jest.spyOn(store, "saveToFile"); + + store.onSync({ a: "foo", b: "", c: "bar" }); + + expect(store.toJSON()).toEqual({ a: "foo", b: "", c: "bar" }); + + expect(fileSpy).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/src/common/base-store.ts b/src/common/base-store.ts index f18efd3d6a..565c3cad89 100644 --- a/src/common/base-store.ts +++ b/src/common/base-store.ts @@ -102,7 +102,7 @@ export abstract class BaseStore extends Singleton { return AppPaths.get("userData"); } - protected async saveToFile(model: T) { + protected saveToFile(model: T) { logger.info(`[STORE]: SAVING ${this.path}`); // todo: update when fixed https://github.com/sindresorhus/conf/issues/114 @@ -166,7 +166,7 @@ export abstract class BaseStore extends Singleton { } } - protected async onModelChange(model: T) { + protected onModelChange(model: T) { if (ipcMain) { this.saveToFile(model); // save config file broadcastMessage(this.syncRendererChannel, model);