Conversation
Backend Unit Tests Coverage Report
|
E2E Workflow Workers Coverage Report
|
E2E Queue Consumers Coverage Report
|
|
E2E SIMS API Coverage Report
|
tiago-graf
added
Devops
| @@ -0,0 +1,14 @@ | |||
| FROM node:20-alpine3.20 | |||
There was a problem hiding this comment.
I tried finding a solution where we wouldn't need a container just to deploy the form definitions but I wasn't happy with the options, for exemple I could expose the formio port and use the make to run the deploy script, but I rather keep the make commands simple and consistent, let me know if you have any ideas!
There was a problem hiding this comment.
Pull request overview
This PR enables Form.io-backed validation during API e2e runs by adding Form.io + MongoDB containers (and a one-shot “forms deploy” step) to the test docker-compose setup, and updates impacted e2e specs to stop relying on mocked FormService.dryRunSubmission.
Changes:
- Added
mongo,formio, andforms-deployservices to the e2e docker-compose stack; wired API startup to wait for form deployment. - Introduced a
Dockerfile.deployimage to deploy JSON form definitions into the Form.io container during CI/e2e. - Updated multiple API e2e tests to send payloads that satisfy real Form.io required fields and removed several dry-run mocks.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| sources/tests/docker-compose.yml | Adds MongoDB + Form.io + forms deployment services and makes API depend on deployed forms. |
| sources/tests/Makefile | Adds env defaults to build Form.io from a configurable upstream repo/tag. |
| sources/tests/.env-e2e-tests | Adds Form.io SA credentials for local e2e execution. |
| sources/packages/forms/Dockerfile.deploy | Adds a deploy-container image to run the form-definition deployment script. |
| sources/packages/backend/libs/test-utils/src/factories/user.ts | Normalizes generated emails to lowercase to reduce case-related mismatches. |
| sources/packages/backend/apps/api/src/testHelpers/index.ts | Removes export of the deleted Form.io mock helper. |
| sources/packages/backend/apps/api/src/route-controllers/supporting-user/tests/e2e/supporting-user.students.controller.submitSupportingUserDetails.e2e-spec.ts | Removes Form.io dry-run mocking and updates payload/assertions for real validation. |
| sources/packages/backend/apps/api/src/route-controllers/student/tests/e2e/student.students.controller.create.e2e-spec.ts | Removes Form.io mocking and adjusts payload values to pass real Form.io validation. |
| sources/packages/backend/apps/api/src/route-controllers/student-scholastic-standings/tests/e2e/*.ts | Removes Form.io mocks and inlines payloads expected to pass/fail real validations. |
| sources/packages/backend/apps/api/src/route-controllers/student-appeal/tests/e2e/*.ts | Switches from mocking to spying on dry-run calls; updates payloads/expectations for real Form.io behavior. |
| sources/packages/backend/apps/api/src/route-controllers//tests/e2e/.ts | Removes Form.io mocks and adjusts test payloads to match current Form.io forms. |
| .github/workflows/repo-checks-tests.yml | Adds Form.io env vars/credentials for CI e2e execution. |
| // Assert | ||
| const updatedSupportingUser = await db.supportingUser.findOne({ | ||
| select: { | ||
| id: true, | ||
| personalInfo: true, | ||
| contactInfo: true, | ||
| supportingData: true, | ||
| modifier: { id: true }, | ||
| }, | ||
| relations: { modifier: true }, | ||
| where: { id: partner.id }, | ||
| }); | ||
| // Assert supporting user reported details. | ||
| // supportingData uses expect.objectContaining because Form.io may return | ||
| // additional computed/hidden fields beyond what the payload submitted. | ||
| expect(updatedSupportingUser).toEqual({ | ||
| id: partner.id, | ||
| contactInfo: { | ||
| address: { | ||
| city: payload.city, | ||
| country: payload.country, | ||
| postalCode: payload.postalCode, | ||
| addressLine1: payload.addressLine1, | ||
| provinceState: payload.provinceState, | ||
| }, | ||
| supportingData: payload.supportingData, | ||
| modifier: { id: student.user.id }, | ||
| }); | ||
| // Assert workflow message. | ||
| expect(zeebeClient.publishMessage).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| correlationKey: partner.id.toString(), | ||
| name: "supporting-user-info-received", | ||
| variables: {}, | ||
| }), | ||
| ); | ||
| }, | ||
| ); | ||
| phone: payload.phone, | ||
| }, | ||
| sin: undefined, | ||
| birthDate: undefined, | ||
| supportingData: { | ||
| totalIncome: payload.supportingData.totalIncome, | ||
| iAgreeToAboveStudentAidBCConsent: | ||
| payload.supportingData.iAgreeToAboveStudentAidBCConsent, | ||
| iAgreeToTheAboveCRAConsent: | ||
| payload.supportingData.iAgreeToTheAboveCRAConsent, | ||
| }, | ||
| supportingUserType: undefined, | ||
| fullName: undefined, | ||
| personalInfo: { | ||
| givenNames: payload.givenNames, | ||
| lastName: payload.lastName, | ||
| }, | ||
| isAbleToReport: undefined, | ||
| user: undefined, | ||
| application: undefined, | ||
| creator: undefined, | ||
| modifier: { | ||
| id: student.user.id, | ||
| userName: undefined, | ||
| email: undefined, | ||
| firstName: undefined, | ||
| lastName: undefined, | ||
| isActive: undefined, | ||
| identityProviderType: undefined, | ||
| creator: undefined, | ||
| modifier: undefined, | ||
| createdAt: undefined, | ||
| updatedAt: undefined, | ||
| }, | ||
| createdAt: undefined, | ||
| updatedAt: undefined, | ||
| }); |
There was a problem hiding this comment.
Same brittleness as the parent case: the expected object includes many unselected properties set to undefined (including nested modifier fields). This can make the test fail due to ORM shape differences rather than behavior. Prefer matching only the selected fields via toMatchObject/objectContaining.
| COPY package.json . | ||
|
|
||
| RUN npm install |
There was a problem hiding this comment.
Docker build is not reproducible because package-lock.json is not copied and the image runs npm install. Since the repo has a package-lock.json, copy it and use npm ci to ensure consistent dependency versions (and typically faster CI builds).
| COPY package.json . | |
| RUN npm install | |
| COPY package.json package-lock.json ./ | |
| RUN npm ci |
| # MongoDB (required by Form.io) | ||
| mongo: | ||
| image: mongo:8.0.11 | ||
| container_name: mongo-${PROJECT_NAME}-${BUILD_REF}-${BUILD_ID} | ||
| networks: | ||
| - local-network-tests | ||
| # - MongoDB | ||
| # Form.io | ||
| formio: | ||
| image: formio-${PROJECT_NAME}:${BUILD_REF}-${BUILD_ID} | ||
| container_name: formio-${PROJECT_NAME}-${BUILD_REF}-${BUILD_ID} | ||
| build: | ||
| context: ../packages/forms | ||
| dockerfile: Dockerfile.dev | ||
| args: | ||
| - FORMIO_SOURCE_REPO_URL=${FORMIO_SOURCE_REPO_URL} | ||
| - FORMIO_SOURCE_REPO_TAG=${FORMIO_SOURCE_REPO_TAG} | ||
| environment: | ||
| NODE_CONFIG: '{"mongo": "mongodb://mongo:27017/formio"}' | ||
| ROOT_EMAIL: ${FORMS_SA_USER_NAME} | ||
| ROOT_PASSWORD: ${FORMS_SA_PASSWORD} | ||
| networks: | ||
| - local-network-tests | ||
| depends_on: | ||
| - mongo | ||
| healthcheck: | ||
| test: ["CMD-SHELL", "wget -qO- http://localhost:3001/access || exit 1"] | ||
| interval: 15s | ||
| timeout: 10s | ||
| retries: 10 | ||
| start_period: 60s | ||
| # - Form.io |
There was a problem hiding this comment.
The formio container does not have a restart policy and only depends_on mongo (no healthcheck). If formio starts before MongoDB is ready and exits, forms-deploy (and therefore api) can be blocked. Consider adding a MongoDB healthcheck + depends_on: condition: service_healthy and/or set formio to restart (as done in sources/forms-docker-compose.yml).
| // Assert | ||
| const updatedSupportingUser = await db.supportingUser.findOne({ | ||
| select: { | ||
| id: true, | ||
| personalInfo: true, | ||
| contactInfo: true, | ||
| supportingData: true, | ||
| modifier: { id: true }, | ||
| }, | ||
| relations: { modifier: true }, | ||
| where: { id: parent.id }, | ||
| }); | ||
| // Assert supporting user reported details. | ||
| // supportingData uses expect.objectContaining because Form.io may return | ||
| // additional computed/hidden fields beyond what the payload submitted. | ||
| expect(updatedSupportingUser).toEqual({ | ||
| id: parent.id, | ||
| contactInfo: { | ||
| address: { | ||
| city: payload.city, | ||
| country: payload.country, | ||
| postalCode: payload.postalCode, | ||
| addressLine1: payload.addressLine1, | ||
| provinceState: payload.provinceState, | ||
| }, | ||
| supportingData: payload.supportingData, | ||
| modifier: { id: student.user.id }, | ||
| }); | ||
| // Assert workflow message. | ||
| expect(zeebeClient.publishMessage).toHaveBeenCalledWith( | ||
| expect.objectContaining({ | ||
| correlationKey: parent.id.toString(), | ||
| name: "supporting-user-info-received", | ||
| variables: {}, | ||
| }), | ||
| ); | ||
| }, | ||
| ); | ||
| phone: payload.phone, | ||
| }, | ||
| sin: undefined, | ||
| birthDate: undefined, | ||
| supportingData: expect.objectContaining(payload.supportingData), | ||
| supportingUserType: undefined, | ||
| fullName: undefined, | ||
| personalInfo: { | ||
| givenNames: payload.givenNames, | ||
| lastName: payload.lastName, | ||
| }, | ||
| isAbleToReport: undefined, | ||
| user: undefined, | ||
| application: undefined, | ||
| creator: undefined, | ||
| modifier: { | ||
| id: student.user.id, | ||
| userName: undefined, | ||
| email: undefined, | ||
| firstName: undefined, | ||
| lastName: undefined, | ||
| isActive: undefined, | ||
| identityProviderType: undefined, | ||
| creator: undefined, | ||
| modifier: undefined, | ||
| createdAt: undefined, | ||
| updatedAt: undefined, | ||
| }, | ||
| createdAt: undefined, | ||
| updatedAt: undefined, | ||
| }); |
There was a problem hiding this comment.
This assertion is very brittle: findOne selects only a few fields, but the expected object includes many additional properties set to undefined (including nested modifier fields). TypeORM usually omits unselected properties rather than populating them, so this can fail even when behavior is correct. Prefer toMatchObject/objectContaining with only the selected fields (id, personalInfo, contactInfo, supportingData, modifier.id).
| supportingData: { | ||
| totalIncome: payload.supportingData.totalIncome, | ||
| iAgreeToAboveStudentAidBCConsent: | ||
| payload.supportingData.iAgreeToAboveStudentAidBCConsent, | ||
| iAgreeToTheAboveCRAConsent: | ||
| payload.supportingData.iAgreeToTheAboveCRAConsent, | ||
| }, |
There was a problem hiding this comment.
The comment says supportingData uses expect.objectContaining, but the assertion uses a strict object literal (only 3 fields). Either update the comment to reflect the intent, or change the expectation to use expect.objectContaining (recommended if Form.io can add/remove computed fields).
| supportingData: { | |
| totalIncome: payload.supportingData.totalIncome, | |
| iAgreeToAboveStudentAidBCConsent: | |
| payload.supportingData.iAgreeToAboveStudentAidBCConsent, | |
| iAgreeToTheAboveCRAConsent: | |
| payload.supportingData.iAgreeToTheAboveCRAConsent, | |
| }, | |
| supportingData: expect.objectContaining({ | |
| totalIncome: payload.supportingData.totalIncome, | |
| iAgreeToAboveStudentAidBCConsent: | |
| payload.supportingData.iAgreeToAboveStudentAidBCConsent, | |
| iAgreeToTheAboveCRAConsent: | |
| payload.supportingData.iAgreeToTheAboveCRAConsent, | |
| }), |
Summary
Adds formio container for e2e tests CICD.
Updates a some e2e tests to use the new container instead of the mocked service.