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

[Refactor] 테스트 계정 생성 개선 #101

Merged
merged 2 commits into from
Jun 20, 2024
Merged

[Refactor] 테스트 계정 생성 개선 #101

merged 2 commits into from
Jun 20, 2024

Conversation

moomint8
Copy link
Member

PR 타입(하나 이상의 PR 타입을 선택해주세요)

  • 기능 추가
  • 기능 삭제
  • 버그 수정
  • 의존성, 환경 변수, 빌드 관련 코드 업데이트
  • 코드 및 구조 개선

반영 브랜치

feat/auth -> dev

변경 사항

테스트 계정 생성 시 더미 값을 이용하던 부분을 전부 없앴습니다.
이제는 더미 값 대신 테스트 용 값을 직접 생성해 테스트합니다.
더 이상 DB에 얽매이지 않고 마음껏 테스트 용 계정을 생성하시길 바랍니다.

moomint8 added 2 commits June 19, 2024 21:03
테스트를 원활히 하기 위한 회원 생성 메소드 중 팀과 직책을 더미 값에 의존하여 더미 값 변경 시 생성이 되지 않는 문제점이 있었습니다.
더미 값과 관계 없는 그 자체의 테스트를 위해 테스트용 본부, 부서, 팀, 직책, 직급을 생성하고 배정하는 방식으로 개선했습니다.
@moomint8 moomint8 added the refactor Improve code or structure label Jun 19, 2024
@moomint8 moomint8 self-assigned this Jun 19, 2024
RIGHT_FORMAT_USER_ROLE, null, null, RIGHT_PHONE_NUMBER, 1, 1,
1);
RIGHT_FORMAT_USER_ROLE, null, null, RIGHT_PHONE_NUMBER, teamId, positionId,
rankId);
}

// 테스트용 관리자 계정 DTO 생성
Copy link

Choose a reason for hiding this comment

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

Code Review:

  1. Bug Risks:

    • No obvious bug risks in the provided code snippet.
  2. Improvement Suggestions:

    • Dependency Injection: Consider abstracting out the service instantiation details, perhaps by using constructor injection instead of field injection. This may improve readability and testability.

    • Error Handling: Include error handling and logging mechanisms to manage exceptions that might occur during the execution of these methods (addCenterForTest, addDepartmentForTest, etc.).

    • Code Duplication: There is some repetition in the addCenterForTest, addDepartmentForTest, addTeamForTest, addPositionForTest, and addRankForTest methods. You might consider refactoring common functionalities into reusable methods to avoid redundancy and improve maintainability.

    • Unit Testing: Write unit tests for the methods such as addCenterForTest, addDepartmentForTest, etc. to ensure they work as expected and handle edge cases adequately.

    • Comments: Comments are beneficial but make sure they add value. Ensure comments explain why something is being done, not just what is being done.

  3. Overall Assessment:

    • The code appears well-structured and organized.
    • The methods seem focused on specific tasks, aiding readability and maintenance.
    • Refactoring and additional error handling could make the code more robust and maintainable.

By implementing these suggestions, you can enhance the code's quality, maintainability, and testability.

Copy link

Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information

Copy link
Collaborator

@Sosohy Sosohy left a comment

Choose a reason for hiding this comment

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

우와 말씀 드리자마자 빨리 수정해주셔서 감사합니다!!
저도 이제 테스트 코드 다시 하나씩 고쳐보겠습니다👍

Copy link
Member

@Leegiyeon Leegiyeon left a comment

Choose a reason for hiding this comment

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

추후 테스트 코드도 개선해보겠습니다..!🤗

Copy link
Collaborator

@kyulin-Kim kyulin-Kim left a comment

Choose a reason for hiding this comment

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

확인했습니다!

고생하셨습니다 :-)

@kyulin-Kim kyulin-Kim merged commit 72c043b into dev Jun 20, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Improve code or structure
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants