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

feat(sprints): created unit tests for the sprints' service and controller methods #224

Merged
merged 20 commits into from
Dec 2, 2024

Conversation

timDeHof
Copy link
Contributor

@timDeHof timDeHof commented Nov 21, 2024

Description

Added comprehensive unit tests for sprints.controller and sprints.service following the testing guidelines. Also created centralized mock data system for improved test maintainability.

Issue link

Fixes # (86azu6kx1)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature updates / changes
  • Tests
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • all unit tests pass
  • verify that mock data consistency
  • check if unit tests followed unit vs e2e test separation rules

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have updated the change log

…ce methods

created a centralized object containing factory functions for basic entities, form-related entities, DTOs, and complex relationships to help with improving maintainability, readability, and consistency.
Mock data is added to facilitate the testing of the sprints service. This includes mock data for various entities such as Voyage, Sprint, TeamMeeting, Form, etc. Helper functions are also provided to create test modules, setup ability checks, and setup Prisma mocks. This will allow for more comprehensive and reliable unit tests, ensuring that the sprints service functions as expected.
…ions to separate file

The mock data and helper functions used in the sprints.service.spec.ts file were extracted to a separate file named mock-data.ts. This change was made to improve the readability and maintainability of the test file. Now, the test file is cleaner and easier to navigate, and the mock data and helper functions can be reused in other test files if needed.
…ontroller

The test coverage was successfully only when directly instantiating the controller for testing
@timDeHof timDeHof self-assigned this Nov 21, 2024
…s to clean up code

Unused imports were removed from both mock-data.ts and sprints.service.spec.ts files. This was done to improve code readability and maintainability by removing clutter. It also helps to reduce the potential for errors and confusion in the future.
The Agenda model from Prisma client is imported to be used in the sprints service tests. This is necessary for testing functionalities that involve the Agenda model.
@timDeHof timDeHof changed the title feat(sprints.service.spec): created unit tests for the sprints' service and controller methods feat(sprints): created unit tests for the sprints' service and controller methods Nov 21, 2024
…d sprints.service.spec.ts

The VoyageResponse import in mock-data.ts and the TestingModule import in sprints.service.spec.ts were removed as they were not being used, which helps to keep the codebase clean and efficient. The MockFormResponseMeetingWithRelations type was also removed from mock-data.ts as it was not being used. In sprints.service.spec.ts, the index parameter was removed from the map function as it was not being used, which simplifies the code and improves readability.
…sponseMeetingWithRelations type and simplify addCheckinFormResponse test

The FormResponseMeetingWithRelations type was removed as it was no longer being used in the codebase, which helps to reduce clutter and improve maintainability. The addCheckinFormResponse test in sprints.service.spec.ts was simplified by removing unnecessary mock data, making the test more focused and easier to understand.
…ture

The test file for the sprints controller has been significantly refactored to improve the structure and increase the test coverage. The changes include:

- Importing necessary modules and dependencies for testing.
- Mocking the SprintsService and its methods.
- Setting up the testing module and initializing the SprintsController and SprintsService before each test.
- Clearing all mocks after each test.
- Adding comprehensive test cases for each method in the SprintsController, including success scenarios and various error scenarios.
- Checking the structure of the response data in some tests to ensure the correct data is returned.

These changes provide a more robust and comprehensive testing suite for the sprints controller, ensuring that it functions as expected in various scenarios.
…rom 'domain'

The 'create' import from 'domain' was not being used in the sprints.controller.spec.ts file. This removal helps to keep the codebase clean and free of unnecessary dependencies.
…for error logging

The change was made to ensure that errors are logged to the error console, which is a more appropriate place for them. This will help in better error tracking and debugging.
@timDeHof timDeHof marked this pull request as ready for review November 25, 2024 02:12
…lity

fix(sprints.service.ts): change console.error to console.log for better error tracking
The indentation in sprints.service.spec.ts was adjusted to improve code readability and maintain consistency across the codebase. In sprints.service.ts, the console.error was changed to console.log to provide better error tracking and debugging. This change will make it easier to identify and fix issues in the future.
The changelog has been updated to include the addition of unit tests for sprints. This is important to keep the changelog up-to-date with all the changes and improvements made to the project, providing a clear history of the project's evolution.
Copy link

@cherylli, @Ajen07
One business day has passed since the review started. Give priority to reviews as much as possible.

@cherylli
Copy link
Contributor

All tests passed but I'm getting this, normal seeding (prisma db seed) is also fine
image

… improve code readability

feat(sprints.service.spec.ts): add consoleSpy to handle database errors during response group creation
The unnecessary blank lines were removed to improve code readability and maintain a clean codebase. A consoleSpy was added to handle database errors during response group creation. This will help in debugging and error tracking, making the application more robust and reliable.
@timDeHof
Copy link
Contributor Author

All tests passed but I'm getting this, normal seeding (prisma db seed) is also fine image

I think this comes from my test in sprints.service, where I test for database errors during response group creation since the addCheckinFormResponse method would console log the error. I have added a jest's spyOn function to catch the errors. The Spy might fail if we change to something other than "log" like "error".

src/sprints/sprints.controller.spec.ts Outdated Show resolved Hide resolved
…ing operators in mockUpdatedMeetingDto

The null coalescing operators were removed from the mockUpdatedMeetingDto object in the sprints.controller.spec.ts file. These operators were unnecessary because the values being coalesced were hardcoded and would never be null. This change simplifies the code and improves readability.
@timDeHof timDeHof requested a review from Ajen07 November 27, 2024 22:02
Ajen07
Ajen07 previously approved these changes Nov 29, 2024
@cherylli
Copy link
Contributor

All tests passed and worked. I'm not requesting any changes but have a few questions/comments

  • "Also created centralized mock data system for improved test maintainability." Does that mean mock-data.ts is a shared file? Should we update the other tests to use this?

@timDeHof
Copy link
Contributor Author

timDeHof commented Dec 1, 2024

All tests passed and worked. I'm not requesting any changes but have a few questions/comments

  • "Also created centralized mock data system for improved test maintainability." Does that mean mock-data.ts is a shared file? Should we update the other tests to use this?

Yes, My idea for mock-data.ts is to be shared file to generate test items.

@cherylli
Copy link
Contributor

cherylli commented Dec 2, 2024

All tests passed and worked. I'm not requesting any changes but have a few questions/comments

  • "Also created centralized mock data system for improved test maintainability." Does that mean mock-data.ts is a shared file? Should we update the other tests to use this?

Yes, My idea for mock-data.ts is to be shared file to generate test items.

In this case, should we move it under some sort of shared folder instead of under /sprint module?

I will also add some low priority tasks to update old tests to use this, more importantly all new tests should use the shared mock data

@timDeHof
Copy link
Contributor Author

timDeHof commented Dec 2, 2024

I'll move the file to the shared folder and let you know when its ready.

…rectory

The mock-data.ts file was moved from the sprints directory to the global/mocks directory. This change was made to improve the organization of the codebase. The mock-data.ts file contains mock data that can be used across multiple tests, not just those in the sprints directory. By moving it to the global/mocks directory, it is now more accessible for use in tests throughout the application.
The mock-data used in sprints.controller.spec.ts and sprints.service.spec.ts was moved to the global mocks directory. This change was made to improve the organization of the codebase, making it easier to locate and reuse mock data across different modules.
@timDeHof
Copy link
Contributor Author

timDeHof commented Dec 2, 2024

I have moved the mock-data.ts to /global/mocks

@timDeHof timDeHof requested a review from Ajen07 December 2, 2024 01:22
Copy link
Contributor

@cherylli cherylli left a comment

Choose a reason for hiding this comment

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

thanks, re ran all the tests with new changes, all passed

src/sprints/sprints.service.spec.ts Show resolved Hide resolved
@cherylli cherylli merged commit d116ff4 into dev Dec 2, 2024
1 check passed
@cherylli cherylli deleted the test/unit-test-sprints branch December 2, 2024 02:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants