From 755a1331da3f72dd2daa1c2b62d0fed26c5ac97d Mon Sep 17 00:00:00 2001 From: Jason Rasmussen Date: Thu, 12 Jan 2023 17:07:57 -0500 Subject: [PATCH] chore(web,server): run code coverage reports (#1313) * chore(web,server): run code coverage reports * chore(tests): fail test check if coverage drops * chore: disable e2e until they are fixed * chore(web): coverage threshold --- .github/workflows/test.yml | 16 +- .gitignore | 1 + .../libs/domain/src/user/user.service.spec.ts | 266 +++++++++++++----- server/package.json | 14 +- web/jest.config.mjs | 19 +- web/package.json | 3 +- 6 files changed, 233 insertions(+), 86 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index db79ec0677..729c48e225 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -6,17 +6,17 @@ on: branches: [main] jobs: - e2e-tests: - name: Run end-to-end test suites + # e2e-tests: + # name: Run end-to-end test suites - runs-on: ubuntu-latest + # runs-on: ubuntu-latest - steps: - - name: Checkout code - uses: actions/checkout@v3 + # steps: + # - name: Checkout code + # uses: actions/checkout@v3 - - name: Run Immich Server E2E Test - run: docker-compose -f ./docker/docker-compose.test.yml --env-file ./docker/.env.test up --abort-on-container-exit --exit-code-from immich-server-test + # - name: Run Immich Server E2E Test + # run: docker-compose -f ./docker/docker-compose.test.yml --env-file ./docker/.env.test up --abort-on-container-exit --exit-code-from immich-server-test server-unit-tests: name: Run server unit test suites and checks diff --git a/.gitignore b/.gitignore index 46e6cb9dd2..58561174d7 100644 --- a/.gitignore +++ b/.gitignore @@ -4,3 +4,4 @@ .idea docker/upload +coverage diff --git a/server/libs/domain/src/user/user.service.spec.ts b/server/libs/domain/src/user/user.service.spec.ts index d2aa81f35a..ebafa8ac68 100644 --- a/server/libs/domain/src/user/user.service.spec.ts +++ b/server/libs/domain/src/user/user.service.spec.ts @@ -6,64 +6,77 @@ import { when } from 'jest-when'; import { UserService } from './user.service'; import { UpdateUserDto } from './dto/update-user.dto'; +const adminUserAuth: AuthUserDto = Object.freeze({ + id: 'admin_id', + email: 'admin@test.com', + isAdmin: true, +}); + +const immichUserAuth: AuthUserDto = Object.freeze({ + id: 'immich_id', + email: 'immich@test.com', + isAdmin: false, +}); + +const adminUser: UserEntity = Object.freeze({ + id: adminUserAuth.id, + email: 'admin@test.com', + password: 'admin_password', + firstName: 'admin_first_name', + lastName: 'admin_last_name', + isAdmin: true, + oauthId: '', + shouldChangePassword: false, + profileImagePath: '', + createdAt: '2021-01-01', + tags: [], +}); + +const immichUser: UserEntity = Object.freeze({ + id: immichUserAuth.id, + email: 'immich@test.com', + password: 'immich_password', + firstName: 'immich_first_name', + lastName: 'immich_last_name', + isAdmin: false, + oauthId: '', + shouldChangePassword: false, + profileImagePath: '', + createdAt: '2021-01-01', + tags: [], +}); + +const updatedImmichUser: UserEntity = Object.freeze({ + id: immichUserAuth.id, + email: 'immich@test.com', + password: 'immich_password', + firstName: 'updated_immich_first_name', + lastName: 'updated_immich_last_name', + isAdmin: false, + oauthId: '', + shouldChangePassword: true, + profileImagePath: '', + createdAt: '2021-01-01', + tags: [], +}); + +const adminUserResponse = Object.freeze({ + id: adminUserAuth.id, + email: 'admin@test.com', + deletedAt: undefined, + firstName: 'admin_first_name', + lastName: 'admin_last_name', + isAdmin: true, + oauthId: '', + shouldChangePassword: false, + profileImagePath: '', + createdAt: '2021-01-01', +}); + describe('UserService', () => { let sut: UserService; let userRepositoryMock: jest.Mocked; - const adminUserAuth: AuthUserDto = Object.freeze({ - id: 'admin_id', - email: 'admin@test.com', - isAdmin: true, - }); - - const immichUserAuth: AuthUserDto = Object.freeze({ - id: 'immich_id', - email: 'immich@test.com', - isAdmin: false, - }); - - const adminUser: UserEntity = Object.freeze({ - id: adminUserAuth.id, - email: 'admin@test.com', - password: 'admin_password', - firstName: 'admin_first_name', - lastName: 'admin_last_name', - isAdmin: true, - oauthId: '', - shouldChangePassword: false, - profileImagePath: '', - createdAt: '2021-01-01', - tags: [], - }); - - const immichUser: UserEntity = Object.freeze({ - id: immichUserAuth.id, - email: 'immich@test.com', - password: 'immich_password', - firstName: 'immich_first_name', - lastName: 'immich_last_name', - isAdmin: false, - oauthId: '', - shouldChangePassword: false, - profileImagePath: '', - createdAt: '2021-01-01', - tags: [], - }); - - const updatedImmichUser: UserEntity = Object.freeze({ - id: immichUserAuth.id, - email: 'immich@test.com', - password: 'immich_password', - firstName: 'updated_immich_first_name', - lastName: 'updated_immich_last_name', - isAdmin: false, - oauthId: '', - shouldChangePassword: true, - profileImagePath: '', - createdAt: '2021-01-01', - tags: [], - }); - beforeEach(() => { userRepositoryMock = { get: jest.fn(), @@ -78,12 +91,86 @@ describe('UserService', () => { }; when(userRepositoryMock.get).calledWith(adminUser.id).mockResolvedValue(adminUser); when(userRepositoryMock.get).calledWith(adminUser.id, undefined).mockResolvedValue(adminUser); + when(userRepositoryMock.get).calledWith(immichUser.id).mockResolvedValue(immichUser); when(userRepositoryMock.get).calledWith(immichUser.id, undefined).mockResolvedValue(immichUser); sut = new UserService(userRepositoryMock); }); - describe('Update user', () => { + describe('getAllUsers', () => { + it('should get all users', async () => { + userRepositoryMock.getList.mockResolvedValue([adminUser]); + + const response = await sut.getAllUsers(adminUserAuth, false); + + expect(userRepositoryMock.getList).toHaveBeenCalledWith({ excludeId: adminUser.id }); + expect(response).toEqual([ + { + id: adminUserAuth.id, + email: 'admin@test.com', + deletedAt: undefined, + firstName: 'admin_first_name', + lastName: 'admin_last_name', + isAdmin: true, + oauthId: '', + shouldChangePassword: false, + profileImagePath: '', + createdAt: '2021-01-01', + }, + ]); + }); + }); + + describe('getUserById', () => { + it('should get a user by id', async () => { + userRepositoryMock.get.mockResolvedValue(adminUser); + + const response = await sut.getUserById(adminUser.id); + + expect(userRepositoryMock.get).toHaveBeenCalledWith(adminUser.id, false); + expect(response).toEqual(adminUserResponse); + }); + + it('should throw an error if a user is not found', async () => { + userRepositoryMock.get.mockResolvedValue(null); + + await expect(sut.getUserById(adminUser.id)).rejects.toBeInstanceOf(NotFoundException); + + expect(userRepositoryMock.get).toHaveBeenCalledWith(adminUser.id, false); + }); + }); + + describe('getUserInfo', () => { + it("should get the auth user's info", async () => { + userRepositoryMock.get.mockResolvedValue(adminUser); + + const response = await sut.getUserInfo(adminUser); + + expect(userRepositoryMock.get).toHaveBeenCalledWith(adminUser.id, undefined); + expect(response).toEqual(adminUserResponse); + }); + + it('should throw an error if a user is not found', async () => { + userRepositoryMock.get.mockResolvedValue(null); + + await expect(sut.getUserInfo(adminUser)).rejects.toBeInstanceOf(BadRequestException); + + expect(userRepositoryMock.get).toHaveBeenCalledWith(adminUser.id, undefined); + }); + }); + + describe('getUserCount', () => { + it('should get the user count', async () => { + userRepositoryMock.getList.mockResolvedValue([adminUser]); + + const response = await sut.getUserCount({}); + + expect(userRepositoryMock.getList).toHaveBeenCalled(); + expect(response).toEqual({ userCount: 1 }); + }); + }); + + describe('update', () => { it('should update user', async () => { const update: UpdateUserDto = { id: immichUser.id, @@ -161,17 +248,7 @@ describe('UserService', () => { await expect(result).rejects.toBeInstanceOf(NotFoundException); }); - }); - describe('Delete user', () => { - it('cannot delete admin user', async () => { - const result = sut.deleteUser(adminUserAuth, adminUserAuth.id); - - await expect(result).rejects.toBeInstanceOf(ForbiddenException); - }); - }); - - describe('Create user', () => { it('should let the admin update himself', async () => { const dto = { id: adminUser.id, shouldChangePassword: true, isAdmin: true }; @@ -190,7 +267,37 @@ describe('UserService', () => { await expect(sut.updateUser(adminUser, dto)).rejects.toBeInstanceOf(BadRequestException); }); + }); + describe('restoreUser', () => { + it('should require an admin', async () => { + when(userRepositoryMock.get).calledWith(adminUser.id, true).mockResolvedValue(adminUser); + await expect(sut.restoreUser(immichUserAuth, adminUser.id)).rejects.toBeInstanceOf(ForbiddenException); + expect(userRepositoryMock.get).toHaveBeenCalledWith(adminUser.id, true); + }); + + it('should require the auth user be an admin', async () => { + await expect(sut.deleteUser(immichUserAuth, adminUserAuth.id)).rejects.toBeInstanceOf(ForbiddenException); + + expect(userRepositoryMock.delete).not.toHaveBeenCalled(); + }); + }); + + describe('deleteUser', () => { + it('cannot delete admin user', async () => { + const result = sut.deleteUser(adminUserAuth, adminUserAuth.id); + + await expect(result).rejects.toBeInstanceOf(ForbiddenException); + }); + + it('should require the auth user be an admin', async () => { + await expect(sut.deleteUser(immichUserAuth, adminUserAuth.id)).rejects.toBeInstanceOf(ForbiddenException); + + expect(userRepositoryMock.delete).not.toHaveBeenCalled(); + }); + }); + + describe('update', () => { it('should not create a user if there is no local admin account', async () => { when(userRepositoryMock.getAdmin).calledWith().mockResolvedValueOnce(null); @@ -204,4 +311,33 @@ describe('UserService', () => { ).rejects.toBeInstanceOf(BadRequestException); }); }); + + describe('createProfileImage', () => { + it('should throw an error if the user does not exist', async () => { + const file = { path: '/profile/path' } as Express.Multer.File; + userRepositoryMock.update.mockResolvedValue({ ...adminUser, profileImagePath: file.path }); + + await sut.createProfileImage(adminUserAuth, file); + + expect(userRepositoryMock.update).toHaveBeenCalledWith(adminUserAuth.id, { profileImagePath: file.path }); + }); + }); + + describe('getUserProfileImage', () => { + it('should throw an error if the user does not exist', async () => { + userRepositoryMock.get.mockResolvedValue(null); + + await expect(sut.getUserProfileImage(adminUserAuth.id)).rejects.toBeInstanceOf(NotFoundException); + + expect(userRepositoryMock.get).toHaveBeenCalledWith(adminUserAuth.id, undefined); + }); + + it('should throw an error if the user does not have a picture', async () => { + userRepositoryMock.get.mockResolvedValue(adminUser); + + await expect(sut.getUserProfileImage(adminUserAuth.id)).rejects.toBeInstanceOf(NotFoundException); + + expect(userRepositoryMock.get).toHaveBeenCalledWith(adminUserAuth.id, undefined); + }); + }); }); diff --git a/server/package.json b/server/package.json index a789cd7462..ed30dba2b7 100644 --- a/server/package.json +++ b/server/package.json @@ -22,7 +22,7 @@ "lint:fix": "npm run lint -- --fix", "check": "tsc --noEmit", "check:code": "npm run format && npm run lint && npm run check", - "check:all": "npm run check:code && npm run test", + "check:all": "npm run check:code && npm run test:cov", "test": "jest", "test:watch": "jest --watch", "test:cov": "jest --coverage", @@ -139,6 +139,18 @@ "**/*.(t|j)s" ], "coverageDirectory": "./coverage", + "coverageThreshold": { + "global": { + "lines": 20, + "statements": 20 + }, + "./libs/domain/": { + "branches": 60, + "functions": 80, + "lines": 80, + "statements": 80 + } + }, "testEnvironment": "node", "roots": [ "/apps/", diff --git a/web/jest.config.mjs b/web/jest.config.mjs index c0dbc1c1b8..a411c5b8dc 100644 --- a/web/jest.config.mjs +++ b/web/jest.config.mjs @@ -20,10 +20,16 @@ export default { // collectCoverage: false, // An array of glob patterns indicating a set of files for which coverage information should be collected - // collectCoverageFrom: undefined, + collectCoverageFrom: ['src/**/*.*', '!src/api/open-api/**'], // The directory where Jest should output its coverage files // coverageDirectory: undefined, + coverageThreshold: { + global: { + lines: 4, + statements: 4 + } + }, // An array of regexp pattern strings used to skip coverage collection // coveragePathIgnorePatterns: [ @@ -76,16 +82,7 @@ export default { // ], // An array of file extensions your modules use - // moduleFileExtensions: [ - // "js", - // "mjs", - // "cjs", - // "jsx", - // "ts", - // "tsx", - // "json", - // "node" - // ], + moduleFileExtensions: ['svelte', 'js', 'ts'], // A map from regular expressions to module names or to arrays of module names that allow to stub out resources with a single module moduleNameMapper: { diff --git a/web/package.json b/web/package.json index a67c5103fb..eabd805ab4 100644 --- a/web/package.json +++ b/web/package.json @@ -9,12 +9,13 @@ "check": "svelte-check --no-tsconfig --fail-on-warnings --ignore \"src/api/open-api\"", "check:watch": "npm run check -- --watch", "check:code": "npm run format && npm run lint && npm run check", - "check:all": "npm run check:code && npm test", + "check:all": "npm run check:code && npm run test:cov", "lint": "eslint . --max-warnings 0", "lint:fix": "npm run lint -- --fix", "format": "prettier --check --plugin-search-dir=. .", "format:fix": "prettier --write --plugin-search-dir=. .", "test": "jest", + "test:cov": "jest --coverage", "test:watch": "npm test -- --watch" }, "devDependencies": {