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

Change the test code to use pytest #124

Merged
merged 32 commits into from
Oct 7, 2024
Merged

Change the test code to use pytest #124

merged 32 commits into from
Oct 7, 2024

Conversation

cjho0316
Copy link
Contributor

@cjho0316 cjho0316 commented Sep 21, 2024

Description

Change existing test to pytest

what I've done in this PR:

  1. add tests/test__get_input.py
  2. add tests/test__help.py
  3. add tests/test__parse_setting.py
  4. add tests/test__run_compare.py
  5. add tests/test_cli.py
  6. add tests/test_common.py
  7. add tests/test_fosslight_scanner.py
  8. change tox.ini

Type of change

Please insert 'x' one of the type of change.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Documentation update
  • Refactoring, Maintenance
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@cjho0316 cjho0316 marked this pull request as ready for review September 27, 2024 16:00
Copy link

@MoonJeWoong MoonJeWoong left a comment

Choose a reason for hiding this comment

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

이 정도면 거의 2, 3단계까지 완료하신 수준인 것 아닌가요? 조용히 작업해주셔서 모르고 있다가 깜짝 놀랐습니다 ㅎㅎ... 특히 mocking 사용해주신 부분은 저도 배워갑니다!

분량이 분량이다보니 리뷰가 이것저것 많이 달린 것 같은데 대부분 사소한 것들인 것 같아서 편하게 봐주시면 될 것 같아요! 궁금하신 점 있으시면 말씀 부탁드립니다. 고생 많으셨어요~

tests/test__get_input.py Outdated Show resolved Hide resolved
tests/test__get_input.py Outdated Show resolved Hide resolved
tests/test__get_input.py Show resolved Hide resolved
tests/test__get_input.py Outdated Show resolved Hide resolved
tests/test__parse_setting.py Outdated Show resolved Hide resolved
tests/test__run_compare.py Outdated Show resolved Hide resolved
tests/test_common.py Outdated Show resolved Hide resolved
tests/test_common.py Outdated Show resolved Hide resolved
tests/test_common.py Outdated Show resolved Hide resolved
tests/test_fosslight_scanner.py Outdated Show resolved Hide resolved
tests/test_common.py Outdated Show resolved Hide resolved
tox.ini Outdated Show resolved Hide resolved
Copy link
Contributor

@YongGoose YongGoose left a comment

Choose a reason for hiding this comment

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

아직 해결하지 못 한 문제가 있다고 하셔서 러프하게 코멘트 달았습니다!
전체적으로 깔끔한 것 같네요 :)

  • PR 설명에서 Type of change Refactoring으로 변경하셔야 할 것 같습니다 :)

Copy link

@s-cu-bot s-cu-bot left a comment

Choose a reason for hiding this comment

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

엄청 열심히 짜신 것 같습니다 ㄷㄷ 수고하셨습니다!!

tests/test__get_input.py Show resolved Hide resolved
Copy link

@ena-isme ena-isme 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

@hkkim2021 hkkim2021 left a comment

Choose a reason for hiding this comment

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

저도 다른 리뷰어분들이 남겨주신 질문들이 궁금합니다. 답변 달아주시면 참고하겠습니다. 수고하셨습니다! :)

@soimkim
Copy link
Contributor

soimkim commented Oct 3, 2024

@cjho0316 , 수정 완료된 커맨트에 답변 남겨주시고, Resolve conversation 클릭해주십시오.
Description 정정 부탁드립니다. test 파일 추가보다 기존 test를 pytest로 변경한다고 작성 부탁드립니다.

requirements.txt Outdated Show resolved Hide resolved
@soimkim soimkim added the chore [PR/Issue] Refactoring, maintenance the code label Oct 6, 2024
@soimkim soimkim changed the title add: test file Change the test code to use pytest Oct 7, 2024
@soimkim soimkim merged commit 7c0173b into fosslight:main Oct 7, 2024
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore [PR/Issue] Refactoring, maintenance the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants