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

Add workaround for Pytest 7.0.0 stacktrace normalization bug #49

Merged
merged 2 commits into from
Mar 9, 2022

Conversation

jiridanek
Copy link
Contributor

@jiridanek
Copy link
Contributor Author

pytest 7.0.1 came out today, containing a fix

@jiridanek
Copy link
Contributor Author

This is no longer necessary. I still think that the changes in the PR are nice and helpful, but now it is not fixing anything. I'll close this in a few days if there is no feedback.

@jiridanek jiridanek force-pushed the jd_2022_02_06_pytest branch from f3190e8 to dac605b Compare February 17, 2022 17:56
@kgiusti kgiusti requested review from kgiusti and ganeshmurthy March 2, 2022 19:44
@kgiusti
Copy link
Contributor

kgiusti commented Mar 9, 2022

The changes LGTM - one question: these changes imply that TestCases should call setUp() instead of using an init method. So going forward should we avoid using init in favor of setUp? thanks.

Copy link
Contributor

@kgiusti kgiusti left a comment

Choose a reason for hiding this comment

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

The changes do improve the code regardless.

@jiridanek
Copy link
Contributor Author

The changes LGTM - one question: these changes imply that TestCases should call setUp() instead of using an init method. So going forward should we avoid using init in favor of setUp? thanks.

I think so. Defining __init__ in TestCase instances is unusual. I wouldn't do it, unless there is a compelling reason. Using setUp when possible avoids having to deal with the __init__'s optional argument, which is a distraction (and scause of the original Pytest problems here).

Going further forward, I'd consider using Pytest ways of doing things, exclusively, with the proviso that some custom work is needed to make integration/system testing convenient (pytest-dev/pytest#9141, but current python unittest suffers from the same problem, or lack of feature, to be precise.)

@jiridanek jiridanek force-pushed the jd_2022_02_06_pytest branch from dac605b to 0c8acc1 Compare March 9, 2022 14:10
jiridanek added a commit to jiridanek/skupper-router that referenced this pull request Mar 9, 2022
…d of __init__ in tests

This started as a workaround for Pytest 7.0.0 stacktrace normalization bug pytest-dev/pytest#9610
@jiridanek jiridanek force-pushed the jd_2022_02_06_pytest branch from 0c8acc1 to 83b2cac Compare March 9, 2022 14:12
jiridanek added a commit to jiridanek/skupper-router that referenced this pull request Mar 9, 2022
…d of __init__ in tests

This started as a workaround for Pytest 7.0.0 stacktrace normalization bug pytest-dev/pytest#9610
@jiridanek jiridanek force-pushed the jd_2022_02_06_pytest branch from 83b2cac to 4c4eaf0 Compare March 9, 2022 15:36
…d of __init__ in tests

This started as a workaround for Pytest 7.0.0 stacktrace normalization bug pytest-dev/pytest#9610
@jiridanek jiridanek force-pushed the jd_2022_02_06_pytest branch from 4c4eaf0 to 367a9ad Compare March 9, 2022 17:05
@jiridanek jiridanek merged commit 97ea276 into skupperproject:main Mar 9, 2022
@jiridanek jiridanek deleted the jd_2022_02_06_pytest branch March 9, 2022 17:48
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