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

DR-750 Add back unit test suite #398

Open
wants to merge 45 commits into
base: develop
Choose a base branch
from
Open

DR-750 Add back unit test suite #398

wants to merge 45 commits into from

Conversation

Aish1990
Copy link
Collaborator

@Aish1990 Aish1990 commented Sep 11, 2024

Steps:

  • Update the current unit tests to make them run (and pass)
  • Add CI integration to run unit tests of every Pull Request
  • Add code-coverage checks to the CI
  • Add more unit tests to achieve higher code coverage

Definition of done: Every Pull-Request should check that the unit tests are passing. The unit tests should achieve a high code coverage (70% or more).

@dmohns dmohns changed the title Dr 750 unit test DR-750 Add back unit test suite Sep 11, 2024
@dmohns
Copy link
Contributor

dmohns commented Sep 11, 2024

Hey @Aish1990 I updated the PR description to be more clear.

Could you address the failing linters?

@dmohns
Copy link
Contributor

dmohns commented Sep 26, 2024

Hey @Aish1990 you have syntax errors in the YAML files. Could you please fix them?

@@ -0,0 +1,127 @@
/* eslint-disable @typescript-eslint/no-empty-function */
Copy link
Contributor

Choose a reason for hiding this comment

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

Please don't use file-wide eslint-disable. If you need to disable it, please do so for the line that cannot be fixed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sir, actually it's like left empty intentionally...

  IsISO31661Alpha2: jest.fn(() => (target: any, propertyKey: string) => {}),
  IsString: jest.fn(() => (target: any, propertyKey: string) => {}),
  IsEmail: jest.fn(() => (target: any, propertyKey: string) => {}),
  IsOptional: jest.fn(() => (target: any, propertyKey: string) => {}),
  IsArray: jest.fn(() => (target: any, propertyKey: string) => {}),
  IsNotEmpty: jest.fn(() => (target: any, propertyKey: string) => {}),

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand. But I'm talking about file-wide ignore.

Please use

jest.mock('class-validator', () => ({
  // eslint-disable-next-line no-empty-function
  IsISO31661Alpha2: jest.fn(() => (target: any, propertyKey: string) => {}),
  
  // eslint-disable-next-line no-empty-function
  IsString: jest.fn(() => (target: any, propertyKey: string) => {}),
  
  // eslint-disable-next-line no-empty-function
  IsEmail: jest.fn(() => (target: any, propertyKey: string) => {}),
  
  // eslint-disable-next-line no-empty-function
  IsOptional: jest.fn(() => (target: any, propertyKey: string) => {}),
  
  // eslint-disable-next-line no-empty-function
  IsArray: jest.fn(() => (target: any, propertyKey: string) => {}),
  
  // eslint-disable-next-line no-empty-function
  IsNotEmpty: jest.fn(() => (target: any, propertyKey: string) => {}),
}));

instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay sir...

@Aish1990 Aish1990 requested a review from dmohns October 3, 2024 11:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants