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 existing tox test to pytest #225

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
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