Skip to content

Refactor existing tox test to pytest #225

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 17 commits into from
Oct 3, 2024
Merged

Conversation

YongGoose
Copy link
Contributor

@YongGoose YongGoose commented Sep 13, 2024

Description

Migrated the test suite from Tox to Pytest for improved readability, flexibility.

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)

Signed-off-by: yongjunhong <kevin0928@naver.com>
Signed-off-by: yongjunhong <kevin0928@naver.com>
Signed-off-by: yongjunhong <kevin0928@naver.com>
- rename test methods
- separate window, ubuntu test codes

Signed-off-by: yongjunhong <kevin0928@naver.com>
Signed-off-by: yongjunhong <kevin0928@naver.com>
Signed-off-by: yongjunhong <kevin0928@naver.com>
@YongGoose YongGoose marked this pull request as ready for review September 26, 2024 05:49
Signed-off-by: yongjunhong <kevin0928@naver.com>

Change path separator

Signed-off-by: yongjunhong <kevin0928@naver.com>
@soimkim soimkim added the chore [PR/Issue] Refactoring, maintenance the code label Sep 26, 2024
Signed-off-by: yongjunhong <kevin0928@naver.com>
Signed-off-by: yongjunhong <kevin0928@naver.com>
Signed-off-by: yongjunhong <kevin0928@naver.com>
Signed-off-by: yongjunhong <kevin0928@naver.com>
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.

안녕하세요 용준님! 리뷰를 사소하게 남겨보았는데 확인하시고 의견 부탁드립니다!

Signed-off-by: yongjunhong <kevin0928@naver.com>
Signed-off-by: yongjunhong <kevin0928@naver.com>
Signed-off-by: yongjunhong <kevin0928@naver.com>
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.

코드 잘 봤습니다:) 수고하셨습니다!

Copy link

@cjho0316 cjho0316 left a comment

Choose a reason for hiding this comment

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

LGTM parametrize 정말 잘 사용하시네요 인상깊게 잘 봤습니다!

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.

mark 추가하는 방식 너무 좋은 것 같습니다. 수고하셨습니다 :)

Signed-off-by: yongjunhong <kevin0928@naver.com>
@soimkim
Copy link
Contributor

soimkim commented Oct 2, 2024

@YongGoose , Description 업데이트 부탁드리며 수정 완료된 커맨트는 Resolve Conversation 클릭해주십시오.

Signed-off-by: yongjunhong <kevin0928@naver.com>
@YongGoose
Copy link
Contributor Author

@YongGoose , Description 업데이트 부탁드리며 수정 완료된 커맨트는 Resolve Conversation 클릭해주십시오.

@soimkim
Description 업데이트 하고 해결된 커멘트 resolve 처리 했습니다!

Signed-off-by: yongjunhong <kevin0928@naver.com>
@@ -1,5 +1,5 @@
flake8==3.9.2
pyinstaller
pyinstaller>=6.10.0
Copy link
Contributor

Choose a reason for hiding this comment

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

pyinstaller 버전을 제한한 사유는 무엇인가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

pyinstaller: error: argument --add-binary: Wrong syntax, should be --add-binary=SOURCE:DEST

pyinstaller에서 에러가 발생하며 위 메시지가 발생하여 구문을 고치고 버전을 제한하게 됐습니다.


생각을 해보니 위 에러는 unix / window 환경 차이의 문제 같기도 합니다..! (조금 더 찾아봐야 할 것 같습니다)

개인적으로 1단계 이후 2,3단계도 개인적으로 진행 해보려고 합니다.
그 때 필요 없다고 판단이 되면 pyinstaller 원상복구 시키고 버전 제한도 삭제하도록 하겠습니다!

@soimkim soimkim merged commit 756d120 into fosslight:main Oct 3, 2024
8 checks passed
# for free to join this conversation on GitHub. Already have an account? # 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