Skip to content
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.

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

[CRT-5] Classes API - E2E tests #59

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
3 changes: 3 additions & 0 deletions backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,11 @@
"ts-pattern": "^5.4.0"
},
"devDependencies": {
"@jest/globals": "^29.7.0",
"@nestjs/cli": "^10.0.0",
"@nestjs/schematics": "^10.0.0",
"@nestjs/testing": "^10.0.0",
"@quramy/jest-prisma-node": "^1.8.1",
"@types/express": "^4.17.17",
"@types/jest": "^29.5.2",
"@types/node": "^20.3.1",
Expand All @@ -57,6 +59,7 @@
"eslint-config-prettier": "^9.0.0",
"eslint-plugin-prettier": "^5.0.0",
"jest": "^29.5.0",
"jest-environment-node": "^29.7.0",
"jest-mock-extended": "^4.0.0-beta1",
"json-schema-to-typescript": "^15.0.2",
"mkdirp": "^3.0.1",
Expand Down
2 changes: 1 addition & 1 deletion backend/src/api/classes/classes.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class ClassesController {
const classes = await this.classesService.listClassesWithTeacher({
// TODO: Implement this properly when auth is available
// TODO: Only allow teachers to see their own classes
where: !!teacherId ? { teacherId } : undefined,
where: teacherId ? { teacherId } : undefined,
});

return classes.map((klass) =>
Expand Down
13 changes: 11 additions & 2 deletions backend/src/api/interceptors/prisma-not-found.interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,17 @@ export class PrismaNotFoundInterceptor implements NestInterceptor {

private isPrismaNotFound(error: unknown): boolean {
return (
error instanceof Prisma.PrismaClientKnownRequestError &&
error.code === Prisma_NotFound_ErrorCode
// normal Prisma error
(error instanceof Prisma.PrismaClientKnownRequestError &&
error.code === Prisma_NotFound_ErrorCode) ||
// jestPrisma-thrown error, untyped
(!!error &&
typeof error == "object" &&
"name" in error &&
error.name === "NotFoundError" &&
"code" in error &&
error.code === Prisma_NotFound_ErrorCode &&
"clientVersion" in error)
pierluca marked this conversation as resolved.
Show resolved Hide resolved
);
}
}
6 changes: 3 additions & 3 deletions backend/test/app.e2e-spec.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { Test, TestingModule } from "@nestjs/testing";
import { INestApplication } from "@nestjs/common";
import * as request from "supertest";
import { AppModule } from "./../src/app.module";
import { INestApplication } from "@nestjs/common";
import { Test, TestingModule } from "@nestjs/testing";
import { AppModule } from "src/app.module";

describe("AppController (e2e)", () => {
let app: INestApplication;
Expand Down
112 changes: 112 additions & 0 deletions backend/test/classes.controller.e2e-spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import { INestApplication } from "@nestjs/common";
import * as request from "supertest";
import { getApp } from "./helper";
import { classes, defaultAdmin, defaultTeacher, users } from "test/seed";

const checkClassesInList = (expectedClasses, returnedClasses): void => {
expect(returnedClasses).toHaveLength(expectedClasses.length);
returnedClasses.forEach((returnedClass) => {
const { teacherId, ...referenceClass } = expectedClasses.find(
(c) => c.id == returnedClass.id,
);
expect(returnedClass).toMatchObject(referenceClass);

const {
type: _ignore_type,
email: _ignore_email,
Comment on lines +15 to +16
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want to assert the absence of those fields instead? (also fine for me if we add a comment and do it later)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

they're there and I literally don't want them in the referenceTeacher :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But longer term, I'd like to have proper matchers for the various DTOs.

Copy link
Contributor

Choose a reason for hiding this comment

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

ah I see ^^

...referenceTeacher
} = users.find((u) => u.id === teacherId)!;
expect(returnedClass.teacher).toMatchObject(referenceTeacher);
});
};
Comment on lines +6 to +21
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can probably be done better, I would like to develop a library of matchers, but I believe it should be "good enough" for now :-)


describe("ClassesController (e2e)", () => {
let app: INestApplication;

beforeEach(async () => {
app = await getApp();
});

test("/classes (GET)", async () => {
const response = await request(app.getHttpServer())
.get("/classes")
.expect(200);

const returnedClasses = response.body;
checkClassesInList(classes, returnedClasses);
});

test("/classes?teacherId (GET)", async () => {
const response = await request(app.getHttpServer())
.get(`/classes/?teacherId=${defaultTeacher.id}`)
.expect(200);

const returnedClasses = response.body;
checkClassesInList(classes, returnedClasses);
});

test("/classes?teacherId for admin (GET)", () => {
return request(app.getHttpServer())
.get(`/classes/?teacherId=${defaultAdmin.id}`)
.expect(200)
.expect([]);
});

test("/classes/:id (GET)", async () => {
const klass = classes[0];
const response = await request(app.getHttpServer())
.get(`/classes/${klass.id}`)
.expect(200);

expect(response.body).toMatchObject({
id: klass.id,
name: klass.name,
teacher: {
id: defaultTeacher.id,
name: defaultTeacher.name,
},
sessions: [],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the seed data doesn't have sessions for now, otherwise this would have to be computed too.

studentCount: 0,
});
});

test("/classes/:id (GET) - not found", async () => {
const response = await request(app.getHttpServer())
.get("/classes/999")
.expect(404);

expect(response.body.message).toBe("Not Found");
});

test("/classes (POST)", async () => {
const dto = { name: "New Class", teacherId: defaultTeacher.id };

const response = await request(app.getHttpServer())
.post("/classes")
.send(dto)
.expect(201);

expect(response.body).toEqual({ id: 1, ...dto });
});

test("/classes/:id (PATCH)", async () => {
const klass = classes[0];
const dto = { name: "Updated Class", teacherId: defaultTeacher.id };

const response = await request(app.getHttpServer())
.patch(`/classes/${klass.id}`)
.send(dto)
.expect(200);

expect(response.body).toEqual({ id: klass.id, ...dto });
});

test("/classes/:id (DELETE)", async () => {
const klass = classes[0];
const response = await request(app.getHttpServer())
.delete(`/classes/${klass.id}`)
.expect(200);

expect(response.body).toEqual(klass);
Comment on lines +81 to +110
Copy link
Contributor

Choose a reason for hiding this comment

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

While at it, why not add a "not found" test for POST, PATCH & DELETE as well? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be helpful indeed ^^

});
});
26 changes: 26 additions & 0 deletions backend/test/helper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { Test, TestingModule } from "@nestjs/testing";
import { INestApplication } from "@nestjs/common";
import { PrismaService } from "src/prisma/prisma.service";
import { PrismaModule } from "src/prisma/prisma.module";
import { ApiModule } from "src/api/api.module";

export const getApp = async (): Promise<INestApplication> => {
const mockPrismaService = {
provide: PrismaService,
useValue: jestPrisma.client,
};

const mockPrismaModule = {
module: PrismaModule,
providers: [mockPrismaService],
global: true,
};

const moduleFixture: TestingModule = await Test.createTestingModule({
imports: [mockPrismaModule, ApiModule],
}).compile();

const app = moduleFixture.createNestApplication();
await app.init();
return app;
};
5 changes: 3 additions & 2 deletions backend/test/jest-e2e.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"moduleFileExtensions": ["js", "json", "ts"],
"rootDir": "..",
"modulePaths": ["<rootDir>"],
"testEnvironment": "node",
"testEnvironment": "@quramy/jest-prisma-node/environment",
"testRegex": ".e2e-spec.ts$",
"transform": {
"^.+\\.(t|j)s$": "ts-jest"
Expand All @@ -13,5 +13,6 @@
"\\.d\\.ts$",
"\\.spec\\.(t|j)s$"
],
"coverageDirectory": "./coverage-e2e"
"coverageDirectory": "./coverage-e2e",
"setupFilesAfterEnv": ["<rootDir>/test/setup-prisma.ts"]
}
31 changes: 31 additions & 0 deletions backend/test/seed/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { Prisma, UserType } from "@prisma/client";

export const defaultAdmin: Prisma.UserUncheckedCreateInput = {
id: 101,
name: "Admin User",
email: "[email protected]",
type: UserType.ADMIN,
};

export const defaultTeacher: Prisma.UserUncheckedCreateInput = {
id: 102,
name: "Teacher User",
email: "[email protected]",
type: UserType.TEACHER,
};

export const users = [defaultAdmin, defaultTeacher];

export const classOne: Prisma.ClassUncheckedCreateInput = {
id: 201,
name: "Class One",
teacherId: defaultTeacher.id!,
};

export const classTwo: Prisma.ClassUncheckedCreateInput = {
id: 202,
name: "Class Two",
teacherId: defaultTeacher.id!,
};

export const classes = [classOne, classTwo];
51 changes: 51 additions & 0 deletions backend/test/setup-prisma.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import "@quramy/jest-prisma-node";
import { PrismaClient } from "@prisma/client";
import { promisify } from "util";
import { exec } from "child_process";
import { classes, users } from "test/seed";

async function seedDatabase(prisma: PrismaClient): Promise<void> {
await Promise.all(
users.map((user) =>
prisma.user.upsert({
where: { id: user.id },
create: user,
update: user,
}),
),
);

await Promise.all(
classes.map((klass) =>
prisma.class.upsert({
where: { id: klass.id },
create: klass,
update: klass,
}),
),
);
}

async function clearDatabase(prisma: PrismaClient): Promise<void> {
await prisma.$executeRaw`
DO
$func$
BEGIN
EXECUTE (
SELECT 'TRUNCATE TABLE ' || string_agg(quote_ident(table_name), ', ') || ' RESTART IDENTITY CASCADE'
FROM information_schema.tables
WHERE table_schema = 'public'
AND table_type = 'BASE TABLE'
);
END
$func$;
`;
}

beforeAll(async () => {
await promisify(exec)("npx prisma db push --accept-data-loss");
const prisma = new PrismaClient();
await prisma.$connect();
await clearDatabase(prisma);
await seedDatabase(prisma);
}, 60000);
3 changes: 2 additions & 1 deletion backend/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
"forceConsistentCasingInFileNames": false,
"noFallthroughCasesInSwitch": false,
"strictPropertyInitialization": true,
"resolveJsonModule": true
"resolveJsonModule": true,
"types": ["@types/jest", "@quramy/jest-prisma-node"],
}
}
26 changes: 26 additions & 0 deletions backend/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1233,6 +1233,29 @@ __metadata:
languageName: node
linkType: hard

"@quramy/jest-prisma-core@npm:^1.8.0":
version: 1.8.1
resolution: "@quramy/jest-prisma-core@npm:1.8.1"
dependencies:
chalk: "npm:^4.0.0"
peerDependencies:
jest: ^28.0.0 || ^29.0.0
checksum: 10c0/37b8c04366b0ee92f43eb72512f157e72abdd4e88871ebde6ea385751b3758b6b5a241055e6f14ee775cc0e865f0efc6152e1c076c6b3afc685a32475b1800ad
languageName: node
linkType: hard

"@quramy/jest-prisma-node@npm:^1.8.1":
version: 1.8.1
resolution: "@quramy/jest-prisma-node@npm:1.8.1"
dependencies:
"@quramy/jest-prisma-core": "npm:^1.8.0"
peerDependencies:
jest: ^28.0.0 || ^29.0.0
jest-environment-node: ^28.0.0 || ^29.0.0
checksum: 10c0/4b3dbe2c3120b5bb57e4b31cfecd929045e5b31a060cb000d158c310103a505ad31b7d018b7c0267b3743aa472ee675b8fccc8f086d2cafdc8af02758105ce44
languageName: node
linkType: hard

"@sinclair/typebox@npm:^0.27.8":
version: 0.27.8
resolution: "@sinclair/typebox@npm:0.27.8"
Expand Down Expand Up @@ -2207,6 +2230,7 @@ __metadata:
version: 0.0.0-use.local
resolution: "backend@workspace:."
dependencies:
"@jest/globals": "npm:^29.7.0"
"@nestjs/cli": "npm:^10.0.0"
"@nestjs/common": "npm:^10.0.0"
"@nestjs/config": "npm:^3.2.3"
Expand All @@ -2217,6 +2241,7 @@ __metadata:
"@nestjs/swagger": "npm:^7.4.2"
"@nestjs/testing": "npm:^10.0.0"
"@prisma/client": "npm:^5.20.0"
"@quramy/jest-prisma-node": "npm:^1.8.1"
"@types/express": "npm:^4.17.17"
"@types/jest": "npm:^29.5.2"
"@types/node": "npm:^20.3.1"
Expand All @@ -2230,6 +2255,7 @@ __metadata:
eslint-config-prettier: "npm:^9.0.0"
eslint-plugin-prettier: "npm:^5.0.0"
jest: "npm:^29.5.0"
jest-environment-node: "npm:^29.7.0"
jest-mock-extended: "npm:^4.0.0-beta1"
json-schema-to-typescript: "npm:^15.0.2"
mkdirp: "npm:^3.0.1"
Expand Down
2 changes: 1 addition & 1 deletion sonar-project.properties
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ sonar.javascript.lcov.reportPaths=./backend/coverage/lcov.info,./backend/coverag
# Exclude test files, type definitions and config files from coverage analysis.
# we also exclude some specific files. the following is a list of files that are excluded with the reason why.
# apps/scratch/src/containers/TargetPane.tsx - this file is a copy from the scratch project (with some redundant functions removed) and is difficult to test for now.
sonar.coverage.exclusions=**/__tests__/**/*,**/__mocks__/**/*,**/*.spec.ts,**/*.spec.tsx,**/*.d.ts,**/.eslintrc.js,**/*.config.js,**/*.config.ts,**/tests/**/*,**/apps/scratch/scripts/**/*,**.storybook/**/*,**/*.story.tsx,**/*.stories.tsx,**/apps/scratch/src/containers/TargetPane.tsx,**/backend/prisma/seed.ts
sonar.coverage.exclusions=**/__tests__/**/*,**/__mocks__/**/*,**/*.spec.ts,**/*.spec.tsx,**/*.e2e-spec.ts,**/*.d.ts,**/.eslintrc.js,**/*.config.js,**/*.config.ts,**/tests/**/*,**/apps/scratch/scripts/**/*,**.storybook/**/*,**/*.story.tsx,**/*.stories.tsx,**/apps/scratch/src/containers/TargetPane.tsx,**/backend/prisma/seed.ts,**/backend/test/**/*

# Exclude test files from duplication analysis
sonar.cpd.exclusions=**/__tests__/**/*,**/*.spec.ts,**/*.spec.tsx
Expand Down