From 2d3221c944ca20543179257b4c77631c75ef0be7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Karol=20S=C3=B3jko?= Date: Tue, 24 May 2022 11:06:17 +0200 Subject: [PATCH] fix: running tests and adding tests to CI & CD (#1047) * fix: running tests and adding tests to CI & CD * fix: yarn.lock * fix: alert service * fix: ts-jest utils import --- .github/workflows/beta.yml | 14 +- .github/workflows/dev.yml | 21 +- .github/workflows/pr.yml | 17 +- .github/workflows/prod.yml | 21 +- .../Components/NoteView/NoteView.test.ts | 190 +++++++++--------- .../Components/NoteView/NoteView.tsx | 15 +- .../Components/NoteView/NoteViewProps.ts | 8 + .../javascripts/Services/AlertService.ts | 3 +- .../javascripts/UIModels/AppState/AppState.ts | 6 +- .../UIModels/AppState/EventSource.ts | 4 + .../javascripts/UIModels/AppState/index.ts | 3 +- app/assets/javascripts/jest.config.js | 8 + package.json | 6 +- yarn.lock | 2 +- 14 files changed, 158 insertions(+), 160 deletions(-) create mode 100644 app/assets/javascripts/Components/NoteView/NoteViewProps.ts create mode 100644 app/assets/javascripts/UIModels/AppState/EventSource.ts diff --git a/.github/workflows/beta.yml b/.github/workflows/beta.yml index cc2afebb3..37a1ebb0a 100644 --- a/.github/workflows/beta.yml +++ b/.github/workflows/beta.yml @@ -6,23 +6,23 @@ on: workflow_dispatch: jobs: - tsc: - name: Check types & lint + test: runs-on: ubuntu-latest - steps: - name: Checkout code uses: actions/checkout@v2 - name: Install dependencies run: yarn install --pure-lockfile - - name: Typescript - run: yarn tsc + - name: Bundle + run: yarn bundle - name: ESLint - run: yarn lint --quiet + run: yarn lint + - name: Test + run: yarn test deploy: runs-on: ubuntu-latest - needs: tsc + needs: test steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/dev.yml b/.github/workflows/dev.yml index 6b9a25618..f1ec79fb1 100644 --- a/.github/workflows/dev.yml +++ b/.github/workflows/dev.yml @@ -10,31 +10,26 @@ on: workflow_dispatch: jobs: - - tsc: - - name: Check types & lint - + test: runs-on: ubuntu-latest - steps: - name: Checkout code uses: actions/checkout@v2 - - name: Install dependencies run: yarn install --pure-lockfile - - - name: Typescript - run: yarn tsc - + - name: Bundle + run: yarn bundle - name: ESLint - run: yarn lint --quiet + run: yarn lint + - name: Test + run: yarn test + deploy: runs-on: ubuntu-latest - needs: tsc + needs: test steps: - uses: actions/checkout@v2 diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index 7ad942059..c48769161 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -7,21 +7,16 @@ on: - main jobs: - - tsc: - + test: runs-on: ubuntu-latest - steps: - - name: Checkout code uses: actions/checkout@v2 - - name: Install dependencies run: yarn install --pure-lockfile - - - name: Typescript - run: yarn tsc - + - name: Bundle + run: yarn bundle - name: ESLint - run: yarn lint --quiet + run: yarn lint + - name: Test + run: yarn test diff --git a/.github/workflows/prod.yml b/.github/workflows/prod.yml index eac8cf30d..83365e653 100644 --- a/.github/workflows/prod.yml +++ b/.github/workflows/prod.yml @@ -9,32 +9,25 @@ on: branches: [ main ] jobs: - - tsc: - - name: Check types & lint - + test: runs-on: ubuntu-latest - steps: - - name: Checkout code uses: actions/checkout@v2 - - name: Install dependencies run: yarn install --pure-lockfile - - - name: Typescript - run: yarn tsc - + - name: Bundle + run: yarn bundle - name: ESLint - run: yarn lint --quiet + run: yarn lint + - name: Test + run: yarn test deploy: runs-on: ubuntu-latest - needs: tsc + needs: test steps: - uses: actions/checkout@v2 diff --git a/app/assets/javascripts/Components/NoteView/NoteView.test.ts b/app/assets/javascripts/Components/NoteView/NoteView.test.ts index 46dac906b..2e6317cce 100644 --- a/app/assets/javascripts/Components/NoteView/NoteView.test.ts +++ b/app/assets/javascripts/Components/NoteView/NoteView.test.ts @@ -2,173 +2,175 @@ * @jest-environment jsdom */ -import { NoteView } from './NoteView' +import { WebApplication } from '@/UIModels/Application' +import { AppState } from '@/UIModels/AppState' +import { NotesState } from '@/UIModels/AppState/NotesState' import { ApplicationEvent, ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction, -} from '@standardnotes/snjs/' + NoteViewController, + SNNote, +} from '@standardnotes/snjs' -describe('editor-view', () => { - let ctrl: NoteView - let setShowProtectedWarningSpy: jest.SpyInstance +import { NoteView } from './NoteView' - beforeEach(() => { - ctrl = new NoteView({} as any) +describe('NoteView', () => { + let noteViewController: NoteViewController + let application: WebApplication + let appState: AppState + let notesState: NotesState - setShowProtectedWarningSpy = jest.spyOn(ctrl, 'setShowProtectedOverlay') - - Object.defineProperties(ctrl, { - application: { - value: { - getAppState: () => { - return { - notes: { - setShowProtectedWarning: jest.fn(), - }, - } - }, - hasProtectionSources: () => true, - authorizeNoteAccess: jest.fn(), - }, - }, - removeComponentsObserver: { - value: jest.fn(), - writable: true, - }, - removeTrashKeyObserver: { - value: jest.fn(), - writable: true, - }, - unregisterComponent: { - value: jest.fn(), - writable: true, - }, - editor: { - value: { - clearNoteChangeListener: jest.fn(), - }, - }, + const createNoteView = () => + new NoteView({ + controller: noteViewController, + application, }) - }) + beforeEach(() => { jest.useFakeTimers() + + noteViewController = {} as jest.Mocked + + notesState = {} as jest.Mocked + notesState.setShowProtectedWarning = jest.fn() + + appState = { + notes: notesState, + } as jest.Mocked + + application = {} as jest.Mocked + application.getAppState = jest.fn().mockReturnValue(appState) + application.hasProtectionSources = jest.fn().mockReturnValue(true) + application.authorizeNoteAccess = jest.fn() }) afterEach(() => { jest.useRealTimers() }) - afterEach(() => { - ctrl.deinit() - }) - describe('note is protected', () => { - beforeEach(() => { - Object.defineProperty(ctrl, 'note', { - value: { - protected: true, - }, - }) - }) - it("should hide the note if at the time of the session expiration the note wasn't edited for longer than the allowed idle time", async () => { - jest - .spyOn(ctrl, 'getSecondsElapsedSinceLastEdit') - .mockImplementation(() => ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction + 5) + const secondsElapsedSinceLastEdit = ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction + 5 - await ctrl.onAppEvent(ApplicationEvent.UnprotectedSessionExpired) + noteViewController.note = { + protected: true, + userModifiedDate: new Date(Date.now() - secondsElapsedSinceLastEdit * 1000), + } as jest.Mocked - expect(setShowProtectedWarningSpy).toHaveBeenCalledWith(true) + await createNoteView().onAppEvent(ApplicationEvent.UnprotectedSessionExpired) + + expect(notesState.setShowProtectedWarning).toHaveBeenCalledWith(true) }) it('should postpone the note hiding by correct time if the time passed after its last modification is less than the allowed idle time', async () => { const secondsElapsedSinceLastEdit = ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction - 3 - Object.defineProperty(ctrl.note, 'userModifiedDate', { - value: new Date(Date.now() - secondsElapsedSinceLastEdit * 1000), - configurable: true, - }) + noteViewController.note = { + protected: true, + userModifiedDate: new Date(Date.now() - secondsElapsedSinceLastEdit * 1000), + } as jest.Mocked - await ctrl.onAppEvent(ApplicationEvent.UnprotectedSessionExpired) + await createNoteView().onAppEvent(ApplicationEvent.UnprotectedSessionExpired) const secondsAfterWhichTheNoteShouldHide = ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction - secondsElapsedSinceLastEdit + jest.advanceTimersByTime((secondsAfterWhichTheNoteShouldHide - 1) * 1000) - expect(setShowProtectedWarningSpy).not.toHaveBeenCalled() + + expect(notesState.setShowProtectedWarning).not.toHaveBeenCalled() jest.advanceTimersByTime(1 * 1000) - expect(setShowProtectedWarningSpy).toHaveBeenCalledWith(true) + + expect(notesState.setShowProtectedWarning).toHaveBeenCalledWith(true) }) it('should postpone the note hiding by correct time if the user continued editing it even after the protection session has expired', async () => { const secondsElapsedSinceLastModification = 3 - Object.defineProperty(ctrl.note, 'userModifiedDate', { - value: new Date(Date.now() - secondsElapsedSinceLastModification * 1000), - configurable: true, - }) - await ctrl.onAppEvent(ApplicationEvent.UnprotectedSessionExpired) + noteViewController.note = { + protected: true, + userModifiedDate: new Date(Date.now() - secondsElapsedSinceLastModification * 1000), + } as jest.Mocked + + await createNoteView().onAppEvent(ApplicationEvent.UnprotectedSessionExpired) let secondsAfterWhichTheNoteShouldHide = ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction - secondsElapsedSinceLastModification jest.advanceTimersByTime((secondsAfterWhichTheNoteShouldHide - 1) * 1000) - // A new modification has just happened - Object.defineProperty(ctrl.note, 'userModifiedDate', { - value: new Date(), - configurable: true, - }) + noteViewController.note = { + protected: true, + userModifiedDate: new Date(), + } as jest.Mocked secondsAfterWhichTheNoteShouldHide = ProposedSecondsToDeferUILevelSessionExpirationDuringActiveInteraction jest.advanceTimersByTime((secondsAfterWhichTheNoteShouldHide - 1) * 1000) - expect(setShowProtectedWarningSpy).not.toHaveBeenCalled() + expect(notesState.setShowProtectedWarning).not.toHaveBeenCalled() jest.advanceTimersByTime(1 * 1000) - expect(setShowProtectedWarningSpy).toHaveBeenCalledWith(true) + expect(notesState.setShowProtectedWarning).toHaveBeenCalledWith(true) }) }) describe('note is unprotected', () => { it('should not call any hiding logic', async () => { - Object.defineProperty(ctrl, 'note', { - value: { - protected: false, - }, - }) - const hideProtectedNoteIfInactiveSpy = jest.spyOn(ctrl, 'hideProtectedNoteIfInactive') + noteViewController.note = { + protected: false, + } as jest.Mocked - await ctrl.onAppEvent(ApplicationEvent.UnprotectedSessionExpired) + await createNoteView().onAppEvent(ApplicationEvent.UnprotectedSessionExpired) - expect(hideProtectedNoteIfInactiveSpy).not.toHaveBeenCalled() + expect(notesState.setShowProtectedWarning).not.toHaveBeenCalled() }) }) describe('dismissProtectedWarning', () => { + beforeEach(() => { + noteViewController.note = { + protected: false, + } as jest.Mocked + }) + describe('the note has protection sources', () => { it('should reveal note contents if the authorization has been passed', async () => { - jest.spyOn(ctrl['application'], 'authorizeNoteAccess').mockImplementation(async () => Promise.resolve(true)) + application.authorizeNoteAccess = jest.fn().mockReturnValue(true) - await ctrl.dismissProtectedWarning() + const noteView = new NoteView({ + controller: noteViewController, + application, + }) - expect(setShowProtectedWarningSpy).toHaveBeenCalledWith(false) + await noteView.dismissProtectedWarning() + + expect(notesState.setShowProtectedWarning).toHaveBeenCalledWith(false) }) it('should not reveal note contents if the authorization has not been passed', async () => { - jest.spyOn(ctrl['application'], 'authorizeNoteAccess').mockImplementation(async () => Promise.resolve(false)) + application.authorizeNoteAccess = jest.fn().mockReturnValue(false) - await ctrl.dismissProtectedWarning() + const noteView = new NoteView({ + controller: noteViewController, + application, + }) - expect(setShowProtectedWarningSpy).not.toHaveBeenCalled() + await noteView.dismissProtectedWarning() + + expect(notesState.setShowProtectedWarning).not.toHaveBeenCalled() }) }) describe('the note does not have protection sources', () => { it('should reveal note contents', async () => { - jest.spyOn(ctrl['application'], 'hasProtectionSources').mockImplementation(() => false) + application.hasProtectionSources = jest.fn().mockReturnValue(false) - await ctrl.dismissProtectedWarning() + const noteView = new NoteView({ + controller: noteViewController, + application, + }) - expect(setShowProtectedWarningSpy).toHaveBeenCalledWith(false) + await noteView.dismissProtectedWarning() + + expect(notesState.setShowProtectedWarning).toHaveBeenCalledWith(false) }) }) }) diff --git a/app/assets/javascripts/Components/NoteView/NoteView.tsx b/app/assets/javascripts/Components/NoteView/NoteView.tsx index 4b3f2bb85..7e44b11be 100644 --- a/app/assets/javascripts/Components/NoteView/NoteView.tsx +++ b/app/assets/javascripts/Components/NoteView/NoteView.tsx @@ -1,4 +1,3 @@ -import { WebApplication } from '@/UIModels/Application' import { createRef, JSX, RefObject } from 'preact' import { ApplicationEvent, @@ -15,8 +14,8 @@ import { PayloadEmitSource, } from '@standardnotes/snjs' import { debounce, isDesktopApplication } from '@/Utils' +import { EventSource } from '../../UIModels/AppState/EventSource' import { KeyboardModifier, KeyboardKey } from '@/Services/IOService' -import { EventSource } from '@/UIModels/AppState' import { STRING_DELETE_PLACEHOLDER_ATTEMPT, STRING_DELETE_LOCKED_ATTEMPT, StringDeleteNote } from '@/Strings' import { confirmDialog } from '@/Services/AlertService' import { PureComponent } from '@/Components/Abstract/PureComponent' @@ -35,6 +34,7 @@ import { transactionForDisassociateComponentWithCurrentNote, } from './TransactionFunctions' import { reloadFont } from './FontFunctions' +import { NoteViewProps } from './NoteViewProps' const MINIMUM_STATUS_DURATION = 400 const TEXTAREA_DEBOUNCE = 100 @@ -78,12 +78,7 @@ type State = { rightResizerOffset: number } -interface Props { - application: WebApplication - controller: NoteViewController -} - -export class NoteView extends PureComponent { +export class NoteView extends PureComponent { readonly controller!: NoteViewController private statusTimeout?: NodeJS.Timeout @@ -101,7 +96,7 @@ export class NoteView extends PureComponent { private editorContentRef: RefObject - constructor(props: Props) { + constructor(props: NoteViewProps) { super(props, props.application) this.controller = props.controller @@ -217,7 +212,7 @@ export class NoteView extends PureComponent { } } - override componentDidUpdate(_prevProps: Props, prevState: State): void { + override componentDidUpdate(_prevProps: NoteViewProps, prevState: State): void { if ( this.state.showProtectedWarning != undefined && prevState.showProtectedWarning !== this.state.showProtectedWarning diff --git a/app/assets/javascripts/Components/NoteView/NoteViewProps.ts b/app/assets/javascripts/Components/NoteView/NoteViewProps.ts new file mode 100644 index 000000000..e5041db0d --- /dev/null +++ b/app/assets/javascripts/Components/NoteView/NoteViewProps.ts @@ -0,0 +1,8 @@ +import { NoteViewController } from '@standardnotes/snjs' + +import { WebApplication } from '@/UIModels/Application' + +export interface NoteViewProps { + application: WebApplication + controller: NoteViewController +} diff --git a/app/assets/javascripts/Services/AlertService.ts b/app/assets/javascripts/Services/AlertService.ts index 0e75f623b..125374d56 100644 --- a/app/assets/javascripts/Services/AlertService.ts +++ b/app/assets/javascripts/Services/AlertService.ts @@ -1,4 +1,5 @@ -import { ButtonType, sanitizeHtmlString, AlertService } from '@standardnotes/snjs' +import { ButtonType, sanitizeHtmlString } from '@standardnotes/snjs' +import { AlertService } from '@standardnotes/services' import { SKAlert } from '@standardnotes/stylekit' /** @returns a promise resolving to true if the user confirmed, false if they canceled */ diff --git a/app/assets/javascripts/UIModels/AppState/AppState.ts b/app/assets/javascripts/UIModels/AppState/AppState.ts index b2ca22606..db628fed9 100644 --- a/app/assets/javascripts/UIModels/AppState/AppState.ts +++ b/app/assets/javascripts/UIModels/AppState/AppState.ts @@ -31,17 +31,13 @@ import { AbstractState } from './AbstractState' import { SelectedItemsState } from './SelectedItemsState' import { ListableContentItem } from '@/Components/ContentListView/Types/ListableContentItem' import { AppStateEvent } from './AppStateEvent' +import { EventSource } from './EventSource' export type PanelResizedData = { panel: string collapsed: boolean } -export enum EventSource { - UserInteraction, - Script, -} - type ObserverCallback = (event: AppStateEvent, data?: unknown) => Promise export class AppState extends AbstractState { diff --git a/app/assets/javascripts/UIModels/AppState/EventSource.ts b/app/assets/javascripts/UIModels/AppState/EventSource.ts new file mode 100644 index 000000000..5deb6f1a5 --- /dev/null +++ b/app/assets/javascripts/UIModels/AppState/EventSource.ts @@ -0,0 +1,4 @@ +export enum EventSource { + UserInteraction, + Script, +} diff --git a/app/assets/javascripts/UIModels/AppState/index.ts b/app/assets/javascripts/UIModels/AppState/index.ts index b9f8688d6..c6f89fe51 100644 --- a/app/assets/javascripts/UIModels/AppState/index.ts +++ b/app/assets/javascripts/UIModels/AppState/index.ts @@ -1,3 +1,4 @@ -export { AppState, EventSource, PanelResizedData } from './AppState' +export { AppState, PanelResizedData } from './AppState' export * from './AppStateEvent' +export * from './EventSource' export * from './PurchaseFlowPane' diff --git a/app/assets/javascripts/jest.config.js b/app/assets/javascripts/jest.config.js index 6be1815b2..f7dfda989 100644 --- a/app/assets/javascripts/jest.config.js +++ b/app/assets/javascripts/jest.config.js @@ -25,4 +25,12 @@ module.exports = { '^.+\\.(ts|tsx)?$': 'ts-jest', '\\.svg$': 'svg-jest', }, + coverageThreshold: { + global: { + branches: 3, + functions: 5, + lines: 21, + statements: 22, + }, + }, } diff --git a/package.json b/package.json index a29412798..ef311e331 100644 --- a/package.json +++ b/package.json @@ -16,8 +16,7 @@ "setup": "bundle install && yarn install --frozen-lockfile && bundle exec rails assets:precompile && yarn bundle", "lint": "eslint --fix app/assets/javascripts", "tsc": "tsc --project app/assets/javascripts/tsconfig.json", - "test": "jest --config app/assets/javascripts/jest.config.js", - "test:coverage": "yarn test --coverage", + "test": "jest --config app/assets/javascripts/jest.config.js --coverage", "prepare": "husky install", "postinstall": "yarn run ncu -loglevel verbose --packageFile package.json", "upgrade:snjs": "ncu -u '@standardnotes/*' && yarn" @@ -73,8 +72,9 @@ "@standardnotes/components": "1.8.1", "@standardnotes/filepicker": "1.16.0", "@standardnotes/icons": "^1.1.7", + "@standardnotes/services": "^1.13.1", "@standardnotes/sncrypto-web": "1.10.1", - "@standardnotes/snjs": "2.113.2", + "@standardnotes/snjs": "^2.113.2", "@standardnotes/stylekit": "5.29.0", "@zip.js/zip.js": "^2.4.10", "mobx": "^6.5.0", diff --git a/yarn.lock b/yarn.lock index fbd33747e..9a8a09ca5 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2402,7 +2402,7 @@ buffer "^6.0.3" libsodium-wrappers "^0.7.9" -"@standardnotes/snjs@2.113.2": +"@standardnotes/snjs@^2.113.2": version "2.113.2" resolved "https://registry.yarnpkg.com/@standardnotes/snjs/-/snjs-2.113.2.tgz#efcd435c7e699397f94caa76273440ff56f3f91f" integrity sha512-P93YfvJJJsntiILezwQUkx0ta5Amjd3Pte9+KkUFcLGBL1pkdtoY5lHc1oreWMElYuPTA+K9WrU6/tWa2OVa5A==