* 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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
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 <[email protected]>
---
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
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov <[email protected]> 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 <[email protected]>
> ---
This patch hasn't changed since v1, right?
It's still:
Reviewed-by: David Gow <[email protected]>
Cheers,
-- David
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov <[email protected]> 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 <[email protected]>
> ---
Reviewed-by: David Gow <[email protected]>
Cheers,
-- David
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov <[email protected]> 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 <[email protected]>
> ---
Thanks: I much prefer this to v1. Having it work the same way as
test_tmpdir is a bonus.
Reviewed-by: David Gow <[email protected]>
Cheers,
-- David
On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov <[email protected]> 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 <[email protected]>
> ---
Looks good, thanks!
Reviewed-by: David Gow <[email protected]>
-- David
On Wed, Dec 2, 2020 at 7:05 PM David Gow <[email protected]> wrote:
>
> On Thu, Dec 3, 2020 at 3:09 AM Daniel Latypov <[email protected]> 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 <[email protected]>
> > ---
>
> This patch hasn't changed since v1, right?
>
> It's still:
> Reviewed-by: David Gow <[email protected]>
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
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov <[email protected]> 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 <[email protected]>
Tested-by: Brendan Higgins <[email protected]>
Acked-by: Brendan Higgins <[email protected]>
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov <[email protected]> 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 <[email protected]>
Tested-by: Brendan Higgins <[email protected]>
Acked-by: Brendan Higgins <[email protected]>
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov <[email protected]> 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 <[email protected]>
Tested-by: Brendan Higgins <[email protected]>
Acked-by: Brendan Higgins <[email protected]>
On Wed, Dec 2, 2020 at 11:09 AM Daniel Latypov <[email protected]> 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 <[email protected]>
Tested-by: Brendan Higgins <[email protected]>
Acked-by: Brendan Higgins <[email protected]>