From ba88183f2655f7f84860a40a7edda07deaa523c8 Mon Sep 17 00:00:00 2001 From: Carlos Santos <4a.santos@gmail.com> Date: Wed, 23 Apr 2025 12:12:57 +0200 Subject: [PATCH] backend: Refactor room API test descriptions for consistency and enhance assertion helpers --- .../api/rooms/bulk-delete-rooms.test.ts | 3 +- .../integration/api/rooms/create-room.test.ts | 35 +---- .../integration/api/rooms/delete-room.test.ts | 3 +- .../api/rooms/garbage-collector.test.ts | 10 +- .../integration/api/rooms/get-room.test.ts | 65 +++------ .../integration/api/rooms/get-rooms.test.ts | 96 +++++-------- .../integration/api/rooms/update-room.test.ts | 2 +- backend/tests/utils/assertion-helpers.ts | 133 ++++++++++++++++++ 8 files changed, 200 insertions(+), 147 deletions(-) diff --git a/backend/tests/integration/api/rooms/bulk-delete-rooms.test.ts b/backend/tests/integration/api/rooms/bulk-delete-rooms.test.ts index 3eff724..f611404 100644 --- a/backend/tests/integration/api/rooms/bulk-delete-rooms.test.ts +++ b/backend/tests/integration/api/rooms/bulk-delete-rooms.test.ts @@ -6,12 +6,11 @@ import { stopTestServer, getRoom, joinFakeParticipant, - sleep, disconnectFakeParticipants, bulkDeleteRooms } from '../../../utils/helpers.js'; -describe('OpenVidu Meet Room API Tests', () => { +describe('Room API Tests', () => { beforeAll(async () => { await startTestServer(); }); diff --git a/backend/tests/integration/api/rooms/create-room.test.ts b/backend/tests/integration/api/rooms/create-room.test.ts index 0be51b2..7d0f78b 100644 --- a/backend/tests/integration/api/rooms/create-room.test.ts +++ b/backend/tests/integration/api/rooms/create-room.test.ts @@ -11,10 +11,11 @@ import { import { UserRole } from '../../../../src/typings/ce/user.js'; import INTERNAL_CONFIG from '../../../../src/config/internal-config.js'; import ms from 'ms'; +import { expectValidRoom } from '../../../utils/assertion-helpers.js'; const ROOMS_PATH = `${INTERNAL_CONFIG.API_BASE_PATH_V1}/rooms`; -describe('OpenVidu Meet Room API Tests', () => { +describe('Room API Tests', () => { const validAutoDeletionDate = Date.now() + ms('2h'); let app: Express; @@ -35,17 +36,7 @@ describe('OpenVidu Meet Room API Tests', () => { const room = await createRoom({ roomIdPrefix: ' Test Room ' }); - expect(room).toHaveProperty('creationDate'); - expect(room).not.toHaveProperty('autoDeletionDate'); - expect(room.roomIdPrefix).toBe('TestRoom'); - expect(room).toHaveProperty('preferences'); - expect(room.preferences).toEqual({ - recordingPreferences: { enabled: true }, - chatPreferences: { enabled: true }, - virtualBackgroundPreferences: { enabled: true } - }); - expect(room).toHaveProperty('moderatorRoomUrl'); - expect(room).toHaveProperty('publisherRoomUrl'); + expectValidRoom(room, 'TestRoom'); }); it('✅ Should create a room with a valid autoDeletionDate', async () => { @@ -54,13 +45,7 @@ describe('OpenVidu Meet Room API Tests', () => { roomIdPrefix: ' .,-------}{¡$#<+My Room *123 ' }); - expect(room).toHaveProperty('creationDate'); - expect(room).toHaveProperty('autoDeletionDate'); - expect(room.autoDeletionDate).toBe(validAutoDeletionDate); - expect(room.roomIdPrefix).toBe('MyRoom123'); - expect(room).toHaveProperty('preferences'); - expect(room).toHaveProperty('moderatorRoomUrl'); - expect(room).toHaveProperty('publisherRoomUrl'); + expectValidRoom(room, 'MyRoom123', validAutoDeletionDate); }); it('✅ Should create a room when sending full valid payload', async () => { @@ -76,17 +61,7 @@ describe('OpenVidu Meet Room API Tests', () => { const room = await createRoom(payload); - expect(room).toHaveProperty('creationDate'); - expect(room).toHaveProperty('autoDeletionDate'); - expect(room.autoDeletionDate).toBe(validAutoDeletionDate); - expect(room.roomIdPrefix).toBe('ExampleRoom'); - expect(room.preferences).toEqual({ - recordingPreferences: { enabled: false }, - chatPreferences: { enabled: false }, - virtualBackgroundPreferences: { enabled: true } - }); - expect(room).toHaveProperty('moderatorRoomUrl'); - expect(room).toHaveProperty('publisherRoomUrl'); + expectValidRoom(room, 'ExampleRoom', validAutoDeletionDate, payload.preferences); }); }); diff --git a/backend/tests/integration/api/rooms/delete-room.test.ts b/backend/tests/integration/api/rooms/delete-room.test.ts index b29eb5d..a2f6805 100644 --- a/backend/tests/integration/api/rooms/delete-room.test.ts +++ b/backend/tests/integration/api/rooms/delete-room.test.ts @@ -12,7 +12,7 @@ import { } from '../../../utils/helpers.js'; import ms from 'ms'; -describe('OpenVidu Meet Room API Tests', () => { +describe('Room API Tests', () => { beforeAll(async () => { await startTestServer(); }); @@ -134,7 +134,6 @@ describe('OpenVidu Meet Room API Tests', () => { disconnectFakeParticipants(); - await sleep(2000); const responseAfterDelete = await getRoom(roomId); expect(responseAfterDelete.status).toBe(404); }); diff --git a/backend/tests/integration/api/rooms/garbage-collector.test.ts b/backend/tests/integration/api/rooms/garbage-collector.test.ts index 1176305..44f3ef4 100644 --- a/backend/tests/integration/api/rooms/garbage-collector.test.ts +++ b/backend/tests/integration/api/rooms/garbage-collector.test.ts @@ -14,7 +14,7 @@ import { import ms from 'ms'; import { setInternalConfig } from '../../../../src/config/internal-config.js'; -describe('OpenVidu Meet Room Garbage Collector Tests', () => { +describe('Room Garbage Collector Tests', () => { beforeAll(async () => { setInternalConfig({ MIN_FUTURE_TIME_FOR_ROOM_AUTODELETION_DATE: '0s' @@ -41,7 +41,7 @@ describe('OpenVidu Meet Room Garbage Collector Tests', () => { expect(response.status).toBe(200); // Wait for auto-deletion date to pass - await sleep(2000); + await sleep('2s'); // Run garbage collector await runRoomGarbageCollector(); @@ -88,7 +88,7 @@ describe('OpenVidu Meet Room Garbage Collector Tests', () => { await joinFakeParticipant(createdRoom.roomId, 'test-participant'); // Wait for the auto-deletion date to pass - await sleep(1000); + await sleep('1s'); // Should mark the room for deletion but not delete it yet await runRoomGarbageCollector(); @@ -102,7 +102,7 @@ describe('OpenVidu Meet Room Garbage Collector Tests', () => { disconnectFakeParticipants(); // Wait to receive webhook room_finished - await sleep(3000); + await sleep('2s'); // Verify that the room is deleted response = await getRoom(createdRoom.roomId); @@ -141,7 +141,7 @@ describe('OpenVidu Meet Room Garbage Collector Tests', () => { ]); // Make sure all rooms are expired - await sleep(2000); + await sleep('2s'); await runRoomGarbageCollector(); diff --git a/backend/tests/integration/api/rooms/get-room.test.ts b/backend/tests/integration/api/rooms/get-room.test.ts index fd99fde..b453a2b 100644 --- a/backend/tests/integration/api/rooms/get-room.test.ts +++ b/backend/tests/integration/api/rooms/get-room.test.ts @@ -1,8 +1,13 @@ import { describe, it, expect, beforeAll, afterAll, afterEach } from '@jest/globals'; import { createRoom, deleteAllRooms, startTestServer, stopTestServer, getRoom } from '../../../utils/helpers.js'; import ms from 'ms'; +import { + expectSuccessRoomResponse, + expectValidRoom, + expectValidRoomWithFields +} from '../../../utils/assertion-helpers.js'; -describe('OpenVidu Meet Room API Tests', () => { +describe('Room API Tests', () => { beforeAll(async () => { await startTestServer(); }); @@ -22,48 +27,28 @@ describe('OpenVidu Meet Room API Tests', () => { roomIdPrefix: 'test-room' }); - const response = await getRoom(createdRoom.roomId); - expect(response.status).toBe(200); - expect(response.body).toBeDefined(); - expect(response.body.roomIdPrefix).toBe('test-room'); + expectValidRoom(createdRoom, 'test-room'); - expect(response.body.roomId).toBe(createdRoom.roomId); - expect(response.body.creationDate).toBeDefined(); - expect(response.body.autoDeletionDate).not.toBeDefined(); - expect(response.body.preferences).toEqual({ - recordingPreferences: { enabled: true }, - chatPreferences: { enabled: true }, - virtualBackgroundPreferences: { enabled: true } - }); - expect(response.body.moderatorRoomUrl).toBeDefined(); - expect(response.body.publisherRoomUrl).toBeDefined(); + const response = await getRoom(createdRoom.roomId); + expectSuccessRoomResponse(response, 'test-room'); }); it('should retrieve a room with custom preferences', async () => { - // Create a room with custom preferences - const createdRoom = await createRoom({ + const payload = { roomIdPrefix: 'custom-prefs', preferences: { - recordingPreferences: { enabled: false }, - chatPreferences: { enabled: false }, - virtualBackgroundPreferences: { enabled: true } + recordingPreferences: { enabled: true }, + chatPreferences: { enabled: true }, + virtualBackgroundPreferences: { enabled: false } } - }); - - // Get the roomId from the created room - const roomId = createdRoom.roomId; + }; + // Create a room with custom preferences + const { roomId } = await createRoom(payload); // Retrieve the room by its ID const response = await getRoom(roomId); - // Verify custom preferences - expect(response.status).toBe(200); - expect(response.body.roomId).toBe(roomId); - expect(response.body.preferences).toEqual({ - recordingPreferences: { enabled: false }, - chatPreferences: { enabled: false }, - virtualBackgroundPreferences: { enabled: true } - }); + expectSuccessRoomResponse(response, 'custom-prefs', undefined, payload.preferences); }); it('should retrieve only specified fields when using fields parameter', async () => { @@ -77,14 +62,8 @@ describe('OpenVidu Meet Room API Tests', () => { // Verify that only the requested fields are returned expect(response.status).toBe(200); - expect(response.body.roomId).toBeDefined(); - expect(response.body.roomIdPrefix).toBeDefined(); - // Other fields should not be present - expect(response.body.creationDate).not.toBeDefined(); - expect(response.body.preferences).not.toBeDefined(); - expect(response.body.moderatorRoomUrl).not.toBeDefined(); - expect(response.body.publisherRoomUrl).not.toBeDefined(); + expectValidRoomWithFields(response.body, ['roomId', 'roomIdPrefix']); }); it('should handle roomId with characters that need sanitization', async () => { @@ -97,9 +76,7 @@ describe('OpenVidu Meet Room API Tests', () => { const response = await getRoom(dirtyRoomId); - // The endpoint should sanitize the roomId and still find the room - expect(response.status).toBe(200); - expect(response.body.roomId).toBe(createdRoom.roomId); + expectSuccessRoomResponse(response, 'test-room'); }); it('should retrieve a room with autoDeletionDate', async () => { @@ -115,9 +92,7 @@ describe('OpenVidu Meet Room API Tests', () => { // Get the room const response = await getRoom(createdRoom.roomId); - // Verify autoDeletionDate - expect(response.status).toBe(200); - expect(response.body.autoDeletionDate).toBe(validAutoDeletionDate); + expectSuccessRoomResponse(response, 'deletion-date', validAutoDeletionDate); }); }); diff --git a/backend/tests/integration/api/rooms/get-rooms.test.ts b/backend/tests/integration/api/rooms/get-rooms.test.ts index 3213a34..b4912a5 100644 --- a/backend/tests/integration/api/rooms/get-rooms.test.ts +++ b/backend/tests/integration/api/rooms/get-rooms.test.ts @@ -1,16 +1,15 @@ -import { describe, it, expect, beforeAll, afterAll, afterEach } from '@jest/globals'; -import { - createRoom, - deleteAllRooms, - assertEmptyRooms, - getRooms, - startTestServer, - stopTestServer, - assertSuccessRoomsResponse -} from '../../../utils/helpers.js'; +import { describe, it, beforeAll, afterAll, afterEach } from '@jest/globals'; +import { createRoom, deleteAllRooms, getRooms, startTestServer, stopTestServer } from '../../../utils/helpers.js'; import ms from 'ms'; +import { + expectSuccessRoomsResponse, + expectValidationError, + expectValidRoom, + expectValidRoomWithFields +} from '../../../utils/assertion-helpers.js'; +import { MeetRoom } from '../../../../src/typings/ce/room.js'; -describe('OpenVidu Meet Room API Tests', () => { +describe('Room API Tests', () => { const validAutoDeletionDate = Date.now() + ms('2h'); beforeAll(async () => { @@ -28,33 +27,23 @@ describe('OpenVidu Meet Room API Tests', () => { describe('List Rooms Tests', () => { it('should return an empty list of rooms', async () => { - await assertEmptyRooms(); + const response = await getRooms(); + + expectSuccessRoomsResponse(response, 0, 10, false, false); }); it('should return a list of rooms', async () => { - await assertEmptyRooms(); - await createRoom({ roomIdPrefix: 'test-room' }); const response = await getRooms(); - const { rooms } = response.body; + expectSuccessRoomsResponse(response, 1, 10, false, false); - assertSuccessRoomsResponse(response, 1, 10, false, false); - expect(rooms[0].roomId).toBeDefined(); - expect(rooms[0].roomId).toContain('test-room'); - expect(rooms[0].creationDate).toBeDefined(); - expect(rooms[0].roomIdPrefix).toBeDefined(); - expect(rooms[0].autoDeletionDate).not.toBeDefined(); - expect(rooms[0].preferences).toBeDefined(); - expect(rooms[0].moderatorRoomUrl).toBeDefined(); - expect(rooms[0].publisherRoomUrl).toBeDefined(); + expectValidRoom(response.body.rooms[0], 'test-room'); }); it('should return a list of rooms applying fields filter', async () => { - await assertEmptyRooms(); - await createRoom({ roomIdPrefix: 'test-room', autoDeletionDate: validAutoDeletionDate @@ -63,24 +52,13 @@ describe('OpenVidu Meet Room API Tests', () => { const response = await getRooms({ fields: 'roomId,createdAt' }); const { rooms } = response.body; - assertSuccessRoomsResponse(response, 1, 10, false, false); + expectSuccessRoomsResponse(response, 1, 10, false, false); - expect(rooms[0].roomId).toBeDefined(); - expect(rooms[0].roomId).toContain('test-room'); - - expect(rooms[0].creationDate).not.toBeDefined(); - expect(rooms[0].roomIdPrefix).not.toBeDefined(); - //CreatedAt does not exist in the room - expect(rooms[0].createdAt).not.toBeDefined(); - expect(rooms[0].autoDeletionDate).not.toBeDefined(); - expect(rooms[0].preferences).not.toBeDefined(); - expect(rooms[0].moderatorRoomUrl).not.toBeDefined(); - expect(rooms[0].publisherRoomUrl).not.toBeDefined(); + expectValidRoomWithFields(rooms[0], ['roomId']); }); it('should return a list of rooms with pagination', async () => { - await assertEmptyRooms(); - const promises = [1, 2, 3, 4, 5, 6].map((i) => { + const promises = [0, 1, 2, 3, 4, 5].map((i) => { return createRoom({ roomIdPrefix: `test-room-${i}`, autoDeletionDate: validAutoDeletionDate @@ -89,60 +67,54 @@ describe('OpenVidu Meet Room API Tests', () => { await Promise.all(promises); let response = await getRooms({ maxItems: 3 }); - const { pagination } = response.body; + let { pagination, rooms } = response.body; - assertSuccessRoomsResponse(response, 3, 3, true, true); + expectSuccessRoomsResponse(response, 3, 3, true, true); + rooms.forEach((room: MeetRoom, i: number) => { + expectValidRoom(room, `test-room-${i}`, validAutoDeletionDate); + }); const nextPageToken = pagination.nextPageToken; response = await getRooms({ maxItems: 3, nextPageToken }); - - assertSuccessRoomsResponse(response, 3, 3, false, false); + ({ pagination, rooms } = response.body); + expectSuccessRoomsResponse(response, 3, 3, false, false); + rooms.forEach((room: MeetRoom, i: number) => { + expectValidRoom(room, `test-room-${i + 3}`, validAutoDeletionDate); + }); }); it('should capped maxItems to the maximum allowed', async () => { const response = await getRooms({ maxItems: 101 }); - assertSuccessRoomsResponse(response, 0, 100, false, false); + expectSuccessRoomsResponse(response, 0, 100, false, false); }); it('should coerce a floating number to an integer for maxItems', async () => { const response = await getRooms({ maxItems: 12.78 }); - assertSuccessRoomsResponse(response, 0, 12, false, false); + expectSuccessRoomsResponse(response, 0, 12, false, false); }); }); describe('List Room Validation failures', () => { it('should fail when maxItems is not a number', async () => { const response = await getRooms({ maxItems: 'not-a-number' }); - - expect(response.status).toBe(422); - expect(response.body.error).toContain('Unprocessable Entity'); - // Check that the error details mention an invalid number. - expect(JSON.stringify(response.body.details)).toContain('Expected number, received nan'); + expectValidationError(response, 'maxItems', 'Expected number, received nan'); }); it('should fail when maxItems is negative', async () => { const response = await getRooms({ maxItems: -1 }); - - expect(response.status).toBe(422); - expect(response.body.error).toContain('Unprocessable Entity'); - expect(JSON.stringify(response.body.details)).toContain('positive number'); + expectValidationError(response, 'maxItems', 'must be a positive number'); }); it('should fail when maxItems is zero', async () => { const response = await getRooms({ maxItems: 0 }); - - expect(response.status).toBe(422); - expect(response.body.error).toContain('Unprocessable Entity'); - expect(JSON.stringify(response.body.details)).toContain('positive number'); + expectValidationError(response, 'maxItems', 'must be a positive number'); }); it('should fail when fields is not a string', async () => { const response = await getRooms({ fields: { invalid: 'data' } }); - expect(response.status).toBe(422); - expect(response.body.error).toContain('Unprocessable Entity'); - expect(JSON.stringify(response.body.details)).toContain('Expected string'); + expectValidationError(response, 'fields', 'Expected string'); }); }); }); diff --git a/backend/tests/integration/api/rooms/update-room.test.ts b/backend/tests/integration/api/rooms/update-room.test.ts index a665e5f..367657a 100644 --- a/backend/tests/integration/api/rooms/update-room.test.ts +++ b/backend/tests/integration/api/rooms/update-room.test.ts @@ -8,7 +8,7 @@ import { updateRoomPreferences } from '../../../utils/helpers.js'; -describe('OpenVidu Meet Room API Tests', () => { +describe('Room API Tests', () => { beforeAll(async () => { await startTestServer(); }); diff --git a/backend/tests/utils/assertion-helpers.ts b/backend/tests/utils/assertion-helpers.ts index 60d5a6a..6bd143c 100644 --- a/backend/tests/utils/assertion-helpers.ts +++ b/backend/tests/utils/assertion-helpers.ts @@ -1,7 +1,140 @@ import { expect } from '@jest/globals'; import INTERNAL_CONFIG from '../../src/config/internal-config'; +import { MeetRoom, MeetRoomPreferences } from '../../src/typings/ce'; const RECORDINGS_PATH = `${INTERNAL_CONFIG.INTERNAL_API_BASE_PATH_V1}/recordings`; +export const expectErrorResponse = ( + response: any, + status = 422, + error = 'Unprocessable Entity', + message = 'Invalid request', + details: Array<{ field?: string; message: string }> +) => { + expect(response.status).toBe(status); + expect(response.body).toMatchObject({ error, message }); + expect(Array.isArray(response.body.details)).toBe(true); + expect(response.body.details).toEqual( + expect.arrayContaining( + details.map((d) => { + const matcher: any = { message: expect.stringContaining(d.message) }; + + if (d.field !== undefined) { + matcher.field = d.field; + } + + return expect.objectContaining(matcher); + }) + ) + ); +}; + +export const expectValidationError = (response: any, path: string, message: string) => { + expectErrorResponse(response, 422, 'Unprocessable Entity', 'Invalid request', [{ field: path, message }]); +}; + +/** + * Asserts that a rooms response matches the expected values for testing purposes. + * Validates the room array length and pagination properties. + * + * @param body - The API response body to validate + * @param expectedRoomLength - The expected number of rooms in the response + * @param expectedMaxItems - The expected maximum number of items in pagination + * @param expectedTruncated - The expected value for pagination.isTruncated flag + * @param expectedNextPageToken - The expected presence of pagination.nextPageToken + * (if true, expects nextPageToken to be defined; + * if false, expects nextPageToken to be undefined) + */ +export const expectSuccessRoomsResponse = ( + response: any, + expectedRoomLength: number, + expectedMaxItems: number, + expectedTruncated: boolean, + expectedNextPageToken: boolean +) => { + const { body } = response; + expect(response.status).toBe(200); + expect(body).toBeDefined(); + expect(body.rooms).toBeDefined(); + expect(Array.isArray(body.rooms)).toBe(true); + expect(body.rooms.length).toBe(expectedRoomLength); + expect(body.pagination).toBeDefined(); + expect(body.pagination.isTruncated).toBe(expectedTruncated); + + expectedNextPageToken + ? expect(body.pagination.nextPageToken).toBeDefined() + : expect(body.pagination.nextPageToken).toBeUndefined(); + expect(body.pagination.maxItems).toBe(expectedMaxItems); +}; + +export const expectSuccessRoomResponse = ( + response: any, + idPrefix: string, + autoDeletionDate?: number, + preferences?: MeetRoomPreferences +) => { + expect(response.status).toBe(200); + expectValidRoom(response.body, idPrefix, autoDeletionDate, preferences); +}; + +export const expectValidRoom = ( + room: MeetRoom, + idPrefix: string, + autoDeletionDate?: number, + preferences?: MeetRoomPreferences, + markedForDeletion?: boolean +) => { + expect(room).toBeDefined(); + + expect(room.roomId).toBeDefined(); + expect(room.roomIdPrefix).toBeDefined(); + expect(room.roomIdPrefix).toBe(idPrefix); + expect(room.roomId).not.toBe(''); + expect(room.roomId).toContain(room.roomIdPrefix); + expect(room.creationDate).toBeDefined(); + + if (autoDeletionDate !== undefined) { + expect(room.autoDeletionDate).toBeDefined(); + expect(room.autoDeletionDate).toBe(autoDeletionDate); + } else { + expect(room.autoDeletionDate).toBeUndefined(); + } + + expect(room.preferences).toBeDefined(); + + if (preferences !== undefined) { + expect(room.preferences).toEqual(preferences); + } else { + expect(room.preferences).toEqual({ + recordingPreferences: { enabled: true }, + chatPreferences: { enabled: true }, + virtualBackgroundPreferences: { enabled: true } + }); + } + + expect(room.moderatorRoomUrl).toBeDefined(); + expect(room.publisherRoomUrl).toBeDefined(); + expect(room.moderatorRoomUrl).toContain(room.roomId); + expect(room.publisherRoomUrl).toContain(room.roomId); + expect(room.markedForDeletion).toBeDefined(); + expect(room.markedForDeletion).toBe(markedForDeletion ?? false); +}; + +export const expectValidRoomWithFields = (room: MeetRoom, fields: string[] = []) => { + expect(room).toBeDefined(); + expectObjectFields(room, fields); +}; + +const expectObjectFields = (obj: any, present: string[] = [], absent: string[] = []) => { + present.forEach((key) => { + expect(obj).toHaveProperty(key); + expect((obj as any)[key]).not.toBeUndefined(); + }); + absent.forEach((key) => { + // Si la propiedad existe, debe ser undefined + expect(Object.prototype.hasOwnProperty.call(obj, key) ? (obj as any)[key] : undefined).toBeUndefined(); + }); +}; + export const expectValidRecordingLocationHeader = (response: any) => { // const locationRegex = new RegExp( // `^http://127\\.0\\.0\\.1:\\d+/+${RECORDINGS_PATH.replace(/\//g, '\\/')}/${recordingId}$`