* [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling
@ 2020-12-02 19:08 Daniel Latypov
2020-12-02 19:08 ` [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test Daniel Latypov
` (4 more replies)
0 siblings, 5 replies; 13+ messages in thread
From: Daniel Latypov @ 2020-12-02 19:08 UTC (permalink / raw)
To: davidgow
Cc: brendanhiggins, linux-kernel, linux-kselftest, skhan, Daniel Latypov
* Stop leaking file objects.
* Use self.addCleanup() to ensure we call cleanup functions even if
setUp() fails.
* use mock.patch.stopall instead of more error-prone manual approach
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
tools/testing/kunit/kunit_tool_test.py | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 497ab51bc170..3fbe1acd531a 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -288,19 +288,17 @@ class StrContains(str):
class KUnitMainTest(unittest.TestCase):
def setUp(self):
path = get_absolute_path('test_data/test_is_test_passed-all_passed.log')
- file = open(path)
- all_passed_log = file.readlines()
- self.print_patch = mock.patch('builtins.print')
- self.print_mock = self.print_patch.start()
+ with open(path) as file:
+ all_passed_log = file.readlines()
+
+ self.print_mock = mock.patch('builtins.print').start()
+ self.addCleanup(mock.patch.stopall)
+
self.linux_source_mock = mock.Mock()
self.linux_source_mock.build_reconfig = mock.Mock(return_value=True)
self.linux_source_mock.build_um_kernel = mock.Mock(return_value=True)
self.linux_source_mock.run_kernel = mock.Mock(return_value=all_passed_log)
- def tearDown(self):
- self.print_patch.stop()
- pass
-
def test_config_passes_args_pass(self):
kunit.main(['config', '--build_dir=.kunit'], self.linux_source_mock)
assert self.linux_source_mock.build_reconfig.call_count == 1
base-commit: 509a15421674b9e1a3e1916939d0d0efd3e578da
--
2.29.2.576.ga3fc446d84-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test
2020-12-02 19:08 [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling Daniel Latypov
@ 2020-12-02 19:08 ` Daniel Latypov
2020-12-03 3:05 ` David Gow
2021-01-14 22:22 ` Brendan Higgins
2020-12-02 19:08 ` [PATCH v2 3/4] kunit: tool: use `with open()` " Daniel Latypov
` (3 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Daniel Latypov @ 2020-12-02 19:08 UTC (permalink / raw)
To: davidgow
Cc: brendanhiggins, linux-kernel, linux-kselftest, skhan, Daniel Latypov
Use self.assertEqual/assertNotEqual() instead.
Besides being more appropriate in a unit test, it'll also give a better
error message by show the unexpected values.
Also
* Delete redundant check of exception types. self.assertRaises does this.
* s/kall/call. There's no reason to name it this way.
* This is probably a misunderstanding from the docs which uses it
since `mock.call` is in scope as `call`.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
tools/testing/kunit/kunit_tool_test.py | 50 +++++++++++++-------------
1 file changed, 24 insertions(+), 26 deletions(-)
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 3fbe1acd531a..3a74e5612cf9 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -301,26 +301,26 @@ class KUnitMainTest(unittest.TestCase):
def test_config_passes_args_pass(self):
kunit.main(['config', '--build_dir=.kunit'], self.linux_source_mock)
- assert self.linux_source_mock.build_reconfig.call_count == 1
- assert self.linux_source_mock.run_kernel.call_count == 0
+ self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
+ self.assertEqual(self.linux_source_mock.run_kernel.call_count, 0)
def test_build_passes_args_pass(self):
kunit.main(['build'], self.linux_source_mock)
- assert self.linux_source_mock.build_reconfig.call_count == 0
+ self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 0)
self.linux_source_mock.build_um_kernel.assert_called_once_with(False, 8, '.kunit', None)
- assert self.linux_source_mock.run_kernel.call_count == 0
+ self.assertEqual(self.linux_source_mock.run_kernel.call_count, 0)
def test_exec_passes_args_pass(self):
kunit.main(['exec'], self.linux_source_mock)
- assert self.linux_source_mock.build_reconfig.call_count == 0
- assert self.linux_source_mock.run_kernel.call_count == 1
+ self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 0)
+ self.assertEqual(self.linux_source_mock.run_kernel.call_count, 1)
self.linux_source_mock.run_kernel.assert_called_once_with(build_dir='.kunit', timeout=300)
self.print_mock.assert_any_call(StrContains('Testing complete.'))
def test_run_passes_args_pass(self):
kunit.main(['run'], self.linux_source_mock)
- assert self.linux_source_mock.build_reconfig.call_count == 1
- assert self.linux_source_mock.run_kernel.call_count == 1
+ self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
+ self.assertEqual(self.linux_source_mock.run_kernel.call_count, 1)
self.linux_source_mock.run_kernel.assert_called_once_with(
build_dir='.kunit', timeout=300)
self.print_mock.assert_any_call(StrContains('Testing complete.'))
@@ -329,35 +329,33 @@ class KUnitMainTest(unittest.TestCase):
self.linux_source_mock.run_kernel = mock.Mock(return_value=[])
with self.assertRaises(SystemExit) as e:
kunit.main(['exec'], self.linux_source_mock)
- assert type(e.exception) == SystemExit
- assert e.exception.code == 1
+ self.assertEqual(e.exception.code, 1)
def test_run_passes_args_fail(self):
self.linux_source_mock.run_kernel = mock.Mock(return_value=[])
with self.assertRaises(SystemExit) as e:
kunit.main(['run'], self.linux_source_mock)
- assert type(e.exception) == SystemExit
- assert e.exception.code == 1
- assert self.linux_source_mock.build_reconfig.call_count == 1
- assert self.linux_source_mock.run_kernel.call_count == 1
+ self.assertEqual(e.exception.code, 1)
+ self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
+ self.assertEqual(self.linux_source_mock.run_kernel.call_count, 1)
self.print_mock.assert_any_call(StrContains(' 0 tests run'))
def test_exec_raw_output(self):
self.linux_source_mock.run_kernel = mock.Mock(return_value=[])
kunit.main(['exec', '--raw_output'], self.linux_source_mock)
- assert self.linux_source_mock.run_kernel.call_count == 1
- for kall in self.print_mock.call_args_list:
- assert kall != mock.call(StrContains('Testing complete.'))
- assert kall != mock.call(StrContains(' 0 tests run'))
+ self.assertEqual(self.linux_source_mock.run_kernel.call_count, 1)
+ for call in self.print_mock.call_args_list:
+ self.assertNotEqual(call, mock.call(StrContains('Testing complete.')))
+ self.assertNotEqual(call, mock.call(StrContains(' 0 tests run')))
def test_run_raw_output(self):
self.linux_source_mock.run_kernel = mock.Mock(return_value=[])
kunit.main(['run', '--raw_output'], self.linux_source_mock)
- assert self.linux_source_mock.build_reconfig.call_count == 1
- assert self.linux_source_mock.run_kernel.call_count == 1
- for kall in self.print_mock.call_args_list:
- assert kall != mock.call(StrContains('Testing complete.'))
- assert kall != mock.call(StrContains(' 0 tests run'))
+ self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
+ self.assertEqual(self.linux_source_mock.run_kernel.call_count, 1)
+ for call in self.print_mock.call_args_list:
+ self.assertNotEqual(call, mock.call(StrContains('Testing complete.')))
+ self.assertNotEqual(call, mock.call(StrContains(' 0 tests run')))
def test_exec_timeout(self):
timeout = 3453
@@ -368,7 +366,7 @@ class KUnitMainTest(unittest.TestCase):
def test_run_timeout(self):
timeout = 3453
kunit.main(['run', '--timeout', str(timeout)], self.linux_source_mock)
- assert self.linux_source_mock.build_reconfig.call_count == 1
+ self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
self.linux_source_mock.run_kernel.assert_called_once_with(
build_dir='.kunit', timeout=timeout)
self.print_mock.assert_any_call(StrContains('Testing complete.'))
@@ -376,7 +374,7 @@ class KUnitMainTest(unittest.TestCase):
def test_run_builddir(self):
build_dir = '.kunit'
kunit.main(['run', '--build_dir=.kunit'], self.linux_source_mock)
- assert self.linux_source_mock.build_reconfig.call_count == 1
+ self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
self.linux_source_mock.run_kernel.assert_called_once_with(
build_dir=build_dir, timeout=300)
self.print_mock.assert_any_call(StrContains('Testing complete.'))
@@ -384,7 +382,7 @@ class KUnitMainTest(unittest.TestCase):
def test_config_builddir(self):
build_dir = '.kunit'
kunit.main(['config', '--build_dir', build_dir], self.linux_source_mock)
- assert self.linux_source_mock.build_reconfig.call_count == 1
+ self.assertEqual(self.linux_source_mock.build_reconfig.call_count, 1)
def test_build_builddir(self):
build_dir = '.kunit'
--
2.29.2.576.ga3fc446d84-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/4] kunit: tool: use `with open()` in unit test
2020-12-02 19:08 [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling Daniel Latypov
2020-12-02 19:08 ` [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test Daniel Latypov
@ 2020-12-02 19:08 ` Daniel Latypov
2020-12-03 3:06 ` David Gow
2021-01-14 22:19 ` Brendan Higgins
2020-12-02 19:08 ` [PATCH v2 4/4] minor: kunit: tool: fix unit test so it can run from non-root dir Daniel Latypov
` (2 subsequent siblings)
4 siblings, 2 replies; 13+ messages in thread
From: Daniel Latypov @ 2020-12-02 19:08 UTC (permalink / raw)
To: davidgow
Cc: brendanhiggins, linux-kernel, linux-kselftest, skhan, Daniel Latypov
The use of manual open() and .close() calls seems to be an attempt to
keep the contents in scope.
But Python doesn't restrict variables like that, so we can introduce new
variables inside of a `with` and use them outside.
Do so to make the code more Pythonic.
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
tools/testing/kunit/kunit_tool_test.py | 33 +++++++++++---------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index 3a74e5612cf9..cf160914bc55 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -100,15 +100,14 @@ class KUnitParserTest(unittest.TestCase):
def test_output_isolated_correctly(self):
log_path = get_absolute_path(
'test_data/test_output_isolated_correctly.log')
- file = open(log_path)
- result = kunit_parser.isolate_kunit_output(file.readlines())
+ with open(log_path) as file:
+ result = kunit_parser.isolate_kunit_output(file.readlines())
self.assertContains('TAP version 14', result)
self.assertContains(' # Subtest: example', result)
self.assertContains(' 1..2', result)
self.assertContains(' ok 1 - example_simple_test', result)
self.assertContains(' ok 2 - example_mock_test', result)
self.assertContains('ok 1 - example', result)
- file.close()
def test_output_with_prefix_isolated_correctly(self):
log_path = get_absolute_path(
@@ -143,42 +142,39 @@ class KUnitParserTest(unittest.TestCase):
def test_parse_successful_test_log(self):
all_passed_log = get_absolute_path(
'test_data/test_is_test_passed-all_passed.log')
- file = open(all_passed_log)
- result = kunit_parser.parse_run_tests(file.readlines())
+ with open(all_passed_log) as file:
+ result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
kunit_parser.TestStatus.SUCCESS,
result.status)
- file.close()
def test_parse_failed_test_log(self):
failed_log = get_absolute_path(
'test_data/test_is_test_passed-failure.log')
- file = open(failed_log)
- result = kunit_parser.parse_run_tests(file.readlines())
+ with open(failed_log) as file:
+ result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
kunit_parser.TestStatus.FAILURE,
result.status)
- file.close()
def test_no_tests(self):
empty_log = get_absolute_path(
'test_data/test_is_test_passed-no_tests_run.log')
- file = open(empty_log)
- result = kunit_parser.parse_run_tests(
- kunit_parser.isolate_kunit_output(file.readlines()))
+ with open(empty_log) as file:
+ result = kunit_parser.parse_run_tests(
+ kunit_parser.isolate_kunit_output(file.readlines()))
self.assertEqual(0, len(result.suites))
self.assertEqual(
kunit_parser.TestStatus.NO_TESTS,
result.status)
- file.close()
def test_no_kunit_output(self):
crash_log = get_absolute_path(
'test_data/test_insufficient_memory.log')
- file = open(crash_log)
print_mock = mock.patch('builtins.print').start()
- result = kunit_parser.parse_run_tests(
- kunit_parser.isolate_kunit_output(file.readlines()))
+ with open(crash_log) as file:
+ result = kunit_parser.parse_run_tests(
+ kunit_parser.isolate_kunit_output(file.readlines()))
print_mock.assert_any_call(StrContains('no tests run!'))
print_mock.stop()
file.close()
@@ -186,12 +182,11 @@ class KUnitParserTest(unittest.TestCase):
def test_crashed_test(self):
crashed_log = get_absolute_path(
'test_data/test_is_test_passed-crash.log')
- file = open(crashed_log)
- result = kunit_parser.parse_run_tests(file.readlines())
+ with open(crashed_log) as file:
+ result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
kunit_parser.TestStatus.TEST_CRASHED,
result.status)
- file.close()
def test_ignores_prefix_printk_time(self):
prefix_log = get_absolute_path(
--
2.29.2.576.ga3fc446d84-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/4] minor: kunit: tool: fix unit test so it can run from non-root dir
2020-12-02 19:08 [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling Daniel Latypov
2020-12-02 19:08 ` [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test Daniel Latypov
2020-12-02 19:08 ` [PATCH v2 3/4] kunit: tool: use `with open()` " Daniel Latypov
@ 2020-12-02 19:08 ` Daniel Latypov
2020-12-03 3:06 ` David Gow
2021-01-14 22:15 ` Brendan Higgins
2020-12-03 3:05 ` [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling David Gow
2021-01-14 22:24 ` Brendan Higgins
4 siblings, 2 replies; 13+ messages in thread
From: Daniel Latypov @ 2020-12-02 19:08 UTC (permalink / raw)
To: davidgow
Cc: brendanhiggins, linux-kernel, linux-kselftest, skhan, Daniel Latypov
Also take this time to rename get_absolute_path() to test_data_path().
1. the name is currently a lie. It gives relative paths, e.g. if I run
from the same dir as the test file, it gives './test_data/<file>'
See https://docs.python.org/3/reference/import.html#__file__, which
doesn't stipulate that implementations provide absolute paths.
2. it's only used for generating paths to tools/testing/kunit/test_data/
So we can tersen things by making it less general.
Cache the absolute path to the test data files per suggestion from [1].
Using relative paths, the tests break because of this code in kunit.py
if get_kernel_root_path():
os.chdir(get_kernel_root_path())
[1] https://lore.kernel.org/linux-kselftest/CABVgOSnH0gz7z5JhRCGyG1wg0zDDBTLoSUCoB-gWMeXLgVTo2w@mail.gmail.com/
Fixes: 5578d008d9e0 ("kunit: tool: fix running kunit_tool from outside kernel tree")
Signed-off-by: Daniel Latypov <dlatypov@google.com>
---
tools/testing/kunit/kunit_tool_test.py | 60 +++++++++++---------------
1 file changed, 24 insertions(+), 36 deletions(-)
diff --git a/tools/testing/kunit/kunit_tool_test.py b/tools/testing/kunit/kunit_tool_test.py
index cf160914bc55..1cd127b225a9 100755
--- a/tools/testing/kunit/kunit_tool_test.py
+++ b/tools/testing/kunit/kunit_tool_test.py
@@ -21,16 +21,18 @@ import kunit_json
import kunit
test_tmpdir = ''
+abs_test_data_dir = ''
def setUpModule():
- global test_tmpdir
+ global test_tmpdir, abs_test_data_dir
test_tmpdir = tempfile.mkdtemp()
+ abs_test_data_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), 'test_data'))
def tearDownModule():
shutil.rmtree(test_tmpdir)
-def get_absolute_path(path):
- return os.path.join(os.path.dirname(__file__), path)
+def test_data_path(path):
+ return os.path.join(abs_test_data_dir, path)
class KconfigTest(unittest.TestCase):
@@ -46,8 +48,7 @@ class KconfigTest(unittest.TestCase):
def test_read_from_file(self):
kconfig = kunit_config.Kconfig()
- kconfig_path = get_absolute_path(
- 'test_data/test_read_from_file.kconfig')
+ kconfig_path = test_data_path('test_read_from_file.kconfig')
kconfig.read_from_file(kconfig_path)
@@ -98,8 +99,7 @@ class KUnitParserTest(unittest.TestCase):
str(needle) + '" not found in "' + str(haystack) + '"!')
def test_output_isolated_correctly(self):
- log_path = get_absolute_path(
- 'test_data/test_output_isolated_correctly.log')
+ log_path = test_data_path('test_output_isolated_correctly.log')
with open(log_path) as file:
result = kunit_parser.isolate_kunit_output(file.readlines())
self.assertContains('TAP version 14', result)
@@ -110,8 +110,7 @@ class KUnitParserTest(unittest.TestCase):
self.assertContains('ok 1 - example', result)
def test_output_with_prefix_isolated_correctly(self):
- log_path = get_absolute_path(
- 'test_data/test_pound_sign.log')
+ log_path = test_data_path('test_pound_sign.log')
with open(log_path) as file:
result = kunit_parser.isolate_kunit_output(file.readlines())
self.assertContains('TAP version 14', result)
@@ -140,8 +139,7 @@ class KUnitParserTest(unittest.TestCase):
self.assertContains('ok 3 - string-stream-test', result)
def test_parse_successful_test_log(self):
- all_passed_log = get_absolute_path(
- 'test_data/test_is_test_passed-all_passed.log')
+ all_passed_log = test_data_path('test_is_test_passed-all_passed.log')
with open(all_passed_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
@@ -149,8 +147,7 @@ class KUnitParserTest(unittest.TestCase):
result.status)
def test_parse_failed_test_log(self):
- failed_log = get_absolute_path(
- 'test_data/test_is_test_passed-failure.log')
+ failed_log = test_data_path('test_is_test_passed-failure.log')
with open(failed_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
@@ -158,8 +155,7 @@ class KUnitParserTest(unittest.TestCase):
result.status)
def test_no_tests(self):
- empty_log = get_absolute_path(
- 'test_data/test_is_test_passed-no_tests_run.log')
+ empty_log = test_data_path('test_is_test_passed-no_tests_run.log')
with open(empty_log) as file:
result = kunit_parser.parse_run_tests(
kunit_parser.isolate_kunit_output(file.readlines()))
@@ -169,8 +165,7 @@ class KUnitParserTest(unittest.TestCase):
result.status)
def test_no_kunit_output(self):
- crash_log = get_absolute_path(
- 'test_data/test_insufficient_memory.log')
+ crash_log = test_data_path('test_insufficient_memory.log')
print_mock = mock.patch('builtins.print').start()
with open(crash_log) as file:
result = kunit_parser.parse_run_tests(
@@ -180,8 +175,7 @@ class KUnitParserTest(unittest.TestCase):
file.close()
def test_crashed_test(self):
- crashed_log = get_absolute_path(
- 'test_data/test_is_test_passed-crash.log')
+ crashed_log = test_data_path('test_is_test_passed-crash.log')
with open(crashed_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
@@ -189,8 +183,7 @@ class KUnitParserTest(unittest.TestCase):
result.status)
def test_ignores_prefix_printk_time(self):
- prefix_log = get_absolute_path(
- 'test_data/test_config_printk_time.log')
+ prefix_log = test_data_path('test_config_printk_time.log')
with open(prefix_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
@@ -199,8 +192,7 @@ class KUnitParserTest(unittest.TestCase):
self.assertEqual('kunit-resource-test', result.suites[0].name)
def test_ignores_multiple_prefixes(self):
- prefix_log = get_absolute_path(
- 'test_data/test_multiple_prefixes.log')
+ prefix_log = test_data_path('test_multiple_prefixes.log')
with open(prefix_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
@@ -209,8 +201,7 @@ class KUnitParserTest(unittest.TestCase):
self.assertEqual('kunit-resource-test', result.suites[0].name)
def test_prefix_mixed_kernel_output(self):
- mixed_prefix_log = get_absolute_path(
- 'test_data/test_interrupted_tap_output.log')
+ mixed_prefix_log = test_data_path('test_interrupted_tap_output.log')
with open(mixed_prefix_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
@@ -219,7 +210,7 @@ class KUnitParserTest(unittest.TestCase):
self.assertEqual('kunit-resource-test', result.suites[0].name)
def test_prefix_poundsign(self):
- pound_log = get_absolute_path('test_data/test_pound_sign.log')
+ pound_log = test_data_path('test_pound_sign.log')
with open(pound_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
@@ -228,7 +219,7 @@ class KUnitParserTest(unittest.TestCase):
self.assertEqual('kunit-resource-test', result.suites[0].name)
def test_kernel_panic_end(self):
- panic_log = get_absolute_path('test_data/test_kernel_panic_interrupt.log')
+ panic_log = test_data_path('test_kernel_panic_interrupt.log')
with open(panic_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
@@ -237,7 +228,7 @@ class KUnitParserTest(unittest.TestCase):
self.assertEqual('kunit-resource-test', result.suites[0].name)
def test_pound_no_prefix(self):
- pound_log = get_absolute_path('test_data/test_pound_no_prefix.log')
+ pound_log = test_data_path('test_pound_no_prefix.log')
with open(pound_log) as file:
result = kunit_parser.parse_run_tests(file.readlines())
self.assertEqual(
@@ -248,7 +239,7 @@ class KUnitParserTest(unittest.TestCase):
class KUnitJsonTest(unittest.TestCase):
def _json_for(self, log_file):
- with(open(get_absolute_path(log_file))) as file:
+ with open(test_data_path(log_file)) as file:
test_result = kunit_parser.parse_run_tests(file)
json_obj = kunit_json.get_json_result(
test_result=test_result,
@@ -258,22 +249,19 @@ class KUnitJsonTest(unittest.TestCase):
return json.loads(json_obj)
def test_failed_test_json(self):
- result = self._json_for(
- 'test_data/test_is_test_passed-failure.log')
+ result = self._json_for('test_is_test_passed-failure.log')
self.assertEqual(
{'name': 'example_simple_test', 'status': 'FAIL'},
result["sub_groups"][1]["test_cases"][0])
def test_crashed_test_json(self):
- result = self._json_for(
- 'test_data/test_is_test_passed-crash.log')
+ result = self._json_for('test_is_test_passed-crash.log')
self.assertEqual(
{'name': 'example_simple_test', 'status': 'ERROR'},
result["sub_groups"][1]["test_cases"][0])
def test_no_tests_json(self):
- result = self._json_for(
- 'test_data/test_is_test_passed-no_tests_run.log')
+ result = self._json_for('test_is_test_passed-no_tests_run.log')
self.assertEqual(0, len(result['sub_groups']))
class StrContains(str):
@@ -282,7 +270,7 @@ class StrContains(str):
class KUnitMainTest(unittest.TestCase):
def setUp(self):
- path = get_absolute_path('test_data/test_is_test_passed-all_passed.log')
+ path = test_data_path('test_is_test_passed-all_passed.log')
with open(path) as file:
all_passed_log = file.readlines()
--
2.29.2.576.ga3fc446d84-goog
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling
2020-12-02 19:08 [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling Daniel Latypov
` (2 preceding siblings ...)
2020-12-02 19:08 ` [PATCH v2 4/4] minor: kunit: tool: fix unit test so it can run from non-root dir Daniel Latypov
@ 2020-12-03 3:05 ` David Gow
2020-12-03 5:15 ` Daniel Latypov
2021-01-14 22:24 ` Brendan Higgins
4 siblings, 1 reply; 13+ messages in thread
From: David Gow @ 2020-12-03 3:05 UTC (permalink / raw)
To: Daniel Latypov
Cc: Brendan Higgins, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> * Stop leaking file objects.
> * Use self.addCleanup() to ensure we call cleanup functions even if
> setUp() fails.
> * use mock.patch.stopall instead of more error-prone manual approach
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---
This patch hasn't changed since v1, right?
It's still:
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test
2020-12-02 19:08 ` [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test Daniel Latypov
@ 2020-12-03 3:05 ` David Gow
2021-01-14 22:22 ` Brendan Higgins
1 sibling, 0 replies; 13+ messages in thread
From: David Gow @ 2020-12-03 3:05 UTC (permalink / raw)
To: Daniel Latypov
Cc: Brendan Higgins, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> Use self.assertEqual/assertNotEqual() instead.
> Besides being more appropriate in a unit test, it'll also give a better
> error message by show the unexpected values.
>
> Also
> * Delete redundant check of exception types. self.assertRaises does this.
> * s/kall/call. There's no reason to name it this way.
> * This is probably a misunderstanding from the docs which uses it
> since `mock.call` is in scope as `call`.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---
Looks good, thanks!
Reviewed-by: David Gow <davidgow@google.com>
-- David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] kunit: tool: use `with open()` in unit test
2020-12-02 19:08 ` [PATCH v2 3/4] kunit: tool: use `with open()` " Daniel Latypov
@ 2020-12-03 3:06 ` David Gow
2021-01-14 22:19 ` Brendan Higgins
1 sibling, 0 replies; 13+ messages in thread
From: David Gow @ 2020-12-03 3:06 UTC (permalink / raw)
To: Daniel Latypov
Cc: Brendan Higgins, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> The use of manual open() and .close() calls seems to be an attempt to
> keep the contents in scope.
> But Python doesn't restrict variables like that, so we can introduce new
> variables inside of a `with` and use them outside.
>
> Do so to make the code more Pythonic.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] minor: kunit: tool: fix unit test so it can run from non-root dir
2020-12-02 19:08 ` [PATCH v2 4/4] minor: kunit: tool: fix unit test so it can run from non-root dir Daniel Latypov
@ 2020-12-03 3:06 ` David Gow
2021-01-14 22:15 ` Brendan Higgins
1 sibling, 0 replies; 13+ messages in thread
From: David Gow @ 2020-12-03 3:06 UTC (permalink / raw)
To: Daniel Latypov
Cc: Brendan Higgins, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> Also take this time to rename get_absolute_path() to test_data_path().
>
> 1. the name is currently a lie. It gives relative paths, e.g. if I run
> from the same dir as the test file, it gives './test_data/<file>'
>
> See https://docs.python.org/3/reference/import.html#__file__, which
> doesn't stipulate that implementations provide absolute paths.
>
> 2. it's only used for generating paths to tools/testing/kunit/test_data/
> So we can tersen things by making it less general.
>
> Cache the absolute path to the test data files per suggestion from [1].
> Using relative paths, the tests break because of this code in kunit.py
> if get_kernel_root_path():
> os.chdir(get_kernel_root_path())
>
> [1] https://lore.kernel.org/linux-kselftest/CABVgOSnH0gz7z5JhRCGyG1wg0zDDBTLoSUCoB-gWMeXLgVTo2w@mail.gmail.com/
>
> Fixes: 5578d008d9e0 ("kunit: tool: fix running kunit_tool from outside kernel tree")
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
> ---
Thanks: I much prefer this to v1. Having it work the same way as
test_tmpdir is a bonus.
Reviewed-by: David Gow <davidgow@google.com>
Cheers,
-- David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling
2020-12-03 3:05 ` [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling David Gow
@ 2020-12-03 5:15 ` Daniel Latypov
0 siblings, 0 replies; 13+ messages in thread
From: Daniel Latypov @ 2020-12-03 5:15 UTC (permalink / raw)
To: David Gow
Cc: Brendan Higgins, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Wed, Dec 2, 2020 at 7:05 PM David Gow <davidgow@google.com> wrote:
>
> On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov <dlatypov@google.com> wrote:
> >
> > * Stop leaking file objects.
> > * Use self.addCleanup() to ensure we call cleanup functions even if
> > setUp() fails.
> > * use mock.patch.stopall instead of more error-prone manual approach
> >
> > Signed-off-by: Daniel Latypov <dlatypov@google.com>
> > ---
>
> This patch hasn't changed since v1, right?
>
> It's still:
> Reviewed-by: David Gow <davidgow@google.com>
Oops, yes. It's entirely unchanged.
The only change to the entire series was a rebase + drop the second
patch in favor of revamping the test_data_path() one.
>
> Cheers,
> -- David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] minor: kunit: tool: fix unit test so it can run from non-root dir
2020-12-02 19:08 ` [PATCH v2 4/4] minor: kunit: tool: fix unit test so it can run from non-root dir Daniel Latypov
2020-12-03 3:06 ` David Gow
@ 2021-01-14 22:15 ` Brendan Higgins
1 sibling, 0 replies; 13+ messages in thread
From: Brendan Higgins @ 2021-01-14 22:15 UTC (permalink / raw)
To: Daniel Latypov
Cc: David Gow, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> Also take this time to rename get_absolute_path() to test_data_path().
>
> 1. the name is currently a lie. It gives relative paths, e.g. if I run
> from the same dir as the test file, it gives './test_data/<file>'
>
> See https://docs.python.org/3/reference/import.html#__file__, which
> doesn't stipulate that implementations provide absolute paths.
>
> 2. it's only used for generating paths to tools/testing/kunit/test_data/
> So we can tersen things by making it less general.
>
> Cache the absolute path to the test data files per suggestion from [1].
> Using relative paths, the tests break because of this code in kunit.py
> if get_kernel_root_path():
> os.chdir(get_kernel_root_path())
>
> [1] https://lore.kernel.org/linux-kselftest/CABVgOSnH0gz7z5JhRCGyG1wg0zDDBTLoSUCoB-gWMeXLgVTo2w@mail.gmail.com/
>
> Fixes: 5578d008d9e0 ("kunit: tool: fix running kunit_tool from outside kernel tree")
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
Tested-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Brendan Higgins <brendanhiggins@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] kunit: tool: use `with open()` in unit test
2020-12-02 19:08 ` [PATCH v2 3/4] kunit: tool: use `with open()` " Daniel Latypov
2020-12-03 3:06 ` David Gow
@ 2021-01-14 22:19 ` Brendan Higgins
1 sibling, 0 replies; 13+ messages in thread
From: Brendan Higgins @ 2021-01-14 22:19 UTC (permalink / raw)
To: Daniel Latypov
Cc: David Gow, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> The use of manual open() and .close() calls seems to be an attempt to
> keep the contents in scope.
> But Python doesn't restrict variables like that, so we can introduce new
> variables inside of a `with` and use them outside.
>
> Do so to make the code more Pythonic.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
Tested-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Brendan Higgins <brendanhiggins@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test
2020-12-02 19:08 ` [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test Daniel Latypov
2020-12-03 3:05 ` David Gow
@ 2021-01-14 22:22 ` Brendan Higgins
1 sibling, 0 replies; 13+ messages in thread
From: Brendan Higgins @ 2021-01-14 22:22 UTC (permalink / raw)
To: Daniel Latypov
Cc: David Gow, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> Use self.assertEqual/assertNotEqual() instead.
> Besides being more appropriate in a unit test, it'll also give a better
> error message by show the unexpected values.
>
> Also
> * Delete redundant check of exception types. self.assertRaises does this.
> * s/kall/call. There's no reason to name it this way.
> * This is probably a misunderstanding from the docs which uses it
> since `mock.call` is in scope as `call`.
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
Tested-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Brendan Higgins <brendanhiggins@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling
2020-12-02 19:08 [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling Daniel Latypov
` (3 preceding siblings ...)
2020-12-03 3:05 ` [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling David Gow
@ 2021-01-14 22:24 ` Brendan Higgins
4 siblings, 0 replies; 13+ messages in thread
From: Brendan Higgins @ 2021-01-14 22:24 UTC (permalink / raw)
To: Daniel Latypov
Cc: David Gow, Linux Kernel Mailing List,
open list:KERNEL SELFTEST FRAMEWORK, Shuah Khan
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov <dlatypov@google.com> wrote:
>
> * Stop leaking file objects.
> * Use self.addCleanup() to ensure we call cleanup functions even if
> setUp() fails.
> * use mock.patch.stopall instead of more error-prone manual approach
>
> Signed-off-by: Daniel Latypov <dlatypov@google.com>
Tested-by: Brendan Higgins <brendanhiggins@google.com>
Acked-by: Brendan Higgins <brendanhiggins@google.com>
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-01-14 22:25 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-02 19:08 [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling Daniel Latypov
2020-12-02 19:08 ` [PATCH v2 2/4] kunit: tool: stop using bare asserts in unit test Daniel Latypov
2020-12-03 3:05 ` David Gow
2021-01-14 22:22 ` Brendan Higgins
2020-12-02 19:08 ` [PATCH v2 3/4] kunit: tool: use `with open()` " Daniel Latypov
2020-12-03 3:06 ` David Gow
2021-01-14 22:19 ` Brendan Higgins
2020-12-02 19:08 ` [PATCH v2 4/4] minor: kunit: tool: fix unit test so it can run from non-root dir Daniel Latypov
2020-12-03 3:06 ` David Gow
2021-01-14 22:15 ` Brendan Higgins
2020-12-03 3:05 ` [PATCH v2 1/4] kunit: tool: fix unit test cleanup handling David Gow
2020-12-03 5:15 ` Daniel Latypov
2021-01-14 22:24 ` Brendan Higgins
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).