On Wed, Feb 23, 2022 at 10:26 PM David Gow davidgow@google.com wrote:
On Fri, Feb 18, 2022 at 4:52 AM Daniel Latypov dlatypov@google.com wrote:
Use a more idiomatic check that a list is non-empty (`if mylist:`) and sinmplify the function body by dedenting and using a dict to map between
Nit: spelling of "simplify". (This is also the first time I've seen
Good catch, fixed.
"dedenting" as a word, which I thought was a typo for a while, too...)
"outdent" is the more proper word here. Afaik programmers invented "dedent", e.g. https://docs.python.org/3/library/textwrap.html#textwrap.dedent
I found "dedent" more obvious and used it enough since that I've forgotten it's not really a word.
the kunit TestStatus enum => KernelCI json status string.
The dict hopefully makes it less likely to have bugs like commit 9a6bb30a8830 ("kunit: tool: fix --json output for skipped tests").
Signed-off-by: Daniel Latypov dlatypov@google.com
Note: this series is based on my earlier set of kunit tool cleanups for 5.18, https://lore.kernel.org/linux-kselftest/20220118190922.1557074-1-dlatypov@go...
There's no interesting semantic dependency, just some boring merge conflicts, specifically with patch #4 there, https://lore.kernel.org/linux-kselftest/20220118190922.1557074-5-dlatypov@go...
Looks good to me. While in general, I think I prefer an extra level of indentation to using "continue", it doesn't worry me either way here.
It's not really a concern here, but I'm always wary given python has significant whitespace. I've seen enough instances of moved code where the indentation wasn't properly updated.
The use of a Dict is definitely an improvement.
Reviewed-by: David Gow davidgow@google.com
-- David
tools/testing/kunit/kunit_json.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/tools/testing/kunit/kunit_json.py b/tools/testing/kunit/kunit_json.py index 24d103049bca..14a480d3308a 100644 --- a/tools/testing/kunit/kunit_json.py +++ b/tools/testing/kunit/kunit_json.py @@ -16,24 +16,24 @@ from typing import Any, Dict
JsonObj = Dict[str, Any]
+_status_map: Dict[TestStatus, str] = {
TestStatus.SUCCESS: "PASS",
TestStatus.SKIPPED: "SKIP",
TestStatus.TEST_CRASHED: "ERROR",
+}
def _get_group_json(test: Test, def_config: str, build_dir: str) -> JsonObj: sub_groups = [] # List[JsonObj] test_cases = [] # List[JsonObj]
for subtest in test.subtests:
if len(subtest.subtests):
if subtest.subtests: sub_group = _get_group_json(subtest, def_config, build_dir) sub_groups.append(sub_group)
else:
test_case = {"name": subtest.name, "status": "FAIL"}
if subtest.status == TestStatus.SUCCESS:
test_case["status"] = "PASS"
elif subtest.status == TestStatus.SKIPPED:
test_case["status"] = "SKIP"
elif subtest.status == TestStatus.TEST_CRASHED:
test_case["status"] = "ERROR"
test_cases.append(test_case)
continue
status = _status_map.get(subtest.status, "FAIL")
test_cases.append({"name": subtest.name, "status": status}) test_group = { "name": test.name,
-- 2.35.1.473.g83b2b277ed-goog