-
Notifications
You must be signed in to change notification settings - Fork 343
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
test status available for tearDown() #5224
test status available for tearDown() #5224
Conversation
The `test.start_logging` was removed from nrunner in 263c817 but the `test.stop_logging` wasn't removed. This commit will fix it. Signed-off-by: Jan Richter <[email protected]>
This commit will split the `test._run_avocado` into the two methods. The `_run_test` and `_tearDown`. Thanks to this change, we can store test status before execution of the tearDown and make it available to the users. Reference: avocado-framework#5217 Signed-off-by: Jan Richter <[email protected]>
@@ -859,7 +859,8 @@ def run_avocado(self): | |||
self._tag_end() | |||
self._report() | |||
self.log.info("") | |||
self._stop_logging() | |||
if self._config.get("run.test_runner") != 'nrunner': | |||
self._stop_logging() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, we could drop this method. We should not keep support to others runners at this point and we don't need to introduce new code just to support the legacy runner.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, I agree with you, we should drop the _start_logging
and _stop_logging
. I just didn't want to do it before avocado-vt #3295, because last time we had compatibility issues after dropping legacy code. So I would like to wait until the compatibility between avocado and avocado-vt will be resolved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sorry for being late, and anyway I would not have been a good reviewer as I am not enough familiar with Avocado internals at this time. |
This fix will be available in avocado 95, if everything goes well it should be released on Feb 7. |
This PR will split the
test._run_avocado
into the two methods. The_run_test
and_tearDown
. Thanks to this change, we can store teststatus before execution of the tearDown and make it available to the
users.
Reference: #5217
Signed-off-by: Jan Richter [email protected]