test_that always yields warning "Calling this method from Suite is deprecated" |
|||
Issue description
Whenever I run a test with test_that, I see this a few times in the logs:
12:33:33 WARNI| /build/kevin/usr/local/build/autotest/server/cros/dynamic_suite/suite.py:959: UserWarning: Calling this method from Suite is deprecated
warnings.warn('Calling this method from Suite is deprecated')
I know autotests are typically full of log spam, but this is pretty useless. Do I care about this deprecation? And if I'm supposed to care, maybe it could at least print out what method "this" is?
With a little more investigation, it seems that test_runner_utils.py is calling test_name_equals_predicate and create_fs_getter.
Looks like some of this was done here:
https://chromium-review.googlesource.com/c/452763/
And this fixed up some, but not all, of the "deprecated" usages:
https://chromium-review.googlesource.com/486219
I'm queueing up some changes to fix the warnings, and to make the deprecation warning clearer.
,
Jun 19 2017
The idea is to provide a better solution for cleaning up code than 1. keeping all functions around for fear of breaking something 2. removing an old function and seeing what, if anything, breaks These messages being annoying is therefore working as intended if it motivates people to go fix the problem, but I welcome suggestions about how the messages can be clearer. The more information is included however, the noisier the messages become. I'm not sure where the right balance is.
,
Jun 19 2017
Lowest hanging fruit: when running a test, the word "this" has no context. I've changed that in one of the above CLs, to print what function is deprecated. You could also give a little more effort on fixing deprecated uses yourself... It looks like even the unit tests [1] don't get it right! Beyond examples (i.e., fix what you can; it'll show up when people grep the git logs) and improving the warning message, maybe there's not much to do. [1] server/cros/dynamic_suite/suite_unittest.py still calls several deprecated Suite.xxx functions?
,
Jun 20 2017
I guess the main issue is fixed. I uploaded this to fix up the last semi-related bit I know of: https://chromium-review.googlesource.com/c/540752/
,
Jun 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/cdcf838fe7f450f7d6022b1dc7282c34466c7238 commit cdcf838fe7f450f7d6022b1dc7282c34466c7238 Author: Brian Norris <briannorris@chromium.org> Date: Thu Jun 22 01:48:04 2017 dynamic_suite: don't use deprecated functions in unit test These were generating warnings. We should probably "do the right thing" in our examples, if we expect anyone else to get it right. BUG= chromium:734662 TEST=./suite_unittest.py Change-Id: Iff55dc53d28b5ea423b4bbc3fa888ed3c8ec4b85 Signed-off-by: Brian Norris <briannorris@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/540752 Reviewed-by: Allen Li <ayatane@chromium.org> [modify] https://crrev.com/cdcf838fe7f450f7d6022b1dc7282c34466c7238/server/cros/dynamic_suite/suite_unittest.py
,
Jun 22 2017
OK, that's all I know to fix for now.
,
Jan 22 2018
|
|||
►
Sign in to add a comment |
|||
Comment 1 by briannorris@chromium.org
, Jun 19 2017