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

Fix log/syslog not being correct when last test fails for given module #1395

Merged
merged 4 commits into from
Aug 13, 2020

Conversation

abdosi
Copy link
Contributor

@abdosi abdosi commented Aug 11, 2020

What/Why I did:-
As reported by this issue PR#box/flaky#128
and PR#pytest-dev/pytest-rerunfailures#51
currently pytest will call session/module tear-down before flaky
can retry the test when last test case fails. And Session/Module will be setup again to run last test
retry.
Because of above behaviour when last test fail for given module
all the logs are lost for all test-case upto that point since when
tear down is called dvs container is destroyed and to run retry instance
DVS container is setup up again and logs only belonging to this instance
of run are captured and overwrite all previous logs.

Workaround to have default dummy pass (as suggested in above Issue also) test case at end always so we
avoid module tear-down and setup again and logs are not lost.

currently pytest will call session/module tear-down before flaky
can retry the test. Session/Module will be setup again to run retry
count.

Because of above behaviour when last test fail for given module
all the logs are lost for all test-case upto that point since when
tear down is called dvs container is destroyed and to run retry instance
DVS container is setup up again and logs only belonging to this instance
of run are captured and overwrite all previous logs.

Workaround to have default dummy pass test case at end always so we
avoid module tear-down and setup again.

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@abdosi abdosi added the DVS ⭐ label Aug 11, 2020
@abdosi abdosi requested review from daall and lguohan August 11, 2020 00:44
Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@abdosi
Copy link
Contributor Author

abdosi commented Aug 12, 2020

retest this please

Copy link
Contributor

@daall daall left a comment

Choose a reason for hiding this comment

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

LGTM, only suggestion is to add 2 spaces b/w the test cases and the dummy test case throughout.

It might also be worth adding a note in the "known issues" part of the README explaining why this case is needed and linking to the flaky issue. :)

Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
Signed-off-by: Abhishek Dosi <abdosi@microsoft.com>
@abdosi
Copy link
Contributor Author

abdosi commented Aug 13, 2020

LGTM, only suggestion is to add 2 spaces b/w the test cases and the dummy test case throughout.

It might also be worth adding a note in the "known issues" part of the README explaining why this case is needed and linking to the flaky issue. :)

@daall Updated.

@abdosi abdosi merged commit adc9f8e into sonic-net:master Aug 13, 2020
@abdosi abdosi deleted the pytest_flaky_fix branch August 13, 2020 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants