* [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups
@ 2023-03-13 9:44 Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 01/14] lava_runner: rename to LavaTestResult Tzung-Bi Shih
` (14 more replies)
0 siblings, 15 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
The series changes cros-ec-tests[1].
It applys some clean-ups and fixes obvious errors.
[1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/cros-ec-tests.git/
Tzung-Bi Shih (14):
lava_runner: rename to LavaTestResult
lava_runner: use TextTestRunner
lava_runner: use TextTestResult
lava_runner: respect to verbosity flag from unittest framework
helpers/sysfs: fix NameError
helpers/mcu: fix ResourceWarning on /dev/cros_ec
cros_ec_pwm: fix ResourceWarning on /sys/kernel/debug/pwm
treewide: use context manager on file objects
cros_ec_pwm: use RE to search EC backlight PWM
helpers/mcu: fix packet too long error
helpers/mcu: fix EC_CMD_GET_FEATURES usage
treewide: remove "r" in open() if reading mode
treewide: don't use "+" operator for constructing paths
treewide: don't use "+" operator for concatenating strings
cros/helpers/kernel.py | 5 ++-
cros/helpers/mcu.py | 57 ++++++++++++++------------------
cros/helpers/sysfs.py | 16 ++++-----
cros/runners/lava_runner.py | 64 +++++++++++++++---------------------
cros/tests/cros_ec_accel.py | 13 ++++----
cros/tests/cros_ec_extcon.py | 11 ++++---
cros/tests/cros_ec_pwm.py | 58 +++++++++++++-------------------
cros/tests/cros_ec_rtc.py | 8 ++---
setup.py | 2 +-
9 files changed, 99 insertions(+), 135 deletions(-)
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 01/14] lava_runner: rename to LavaTestResult
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 02/14] lava_runner: use TextTestRunner Tzung-Bi Shih
` (13 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
To simplify, rename from LavaTextTestResult to LavaTestResult.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/runners/lava_runner.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cros/runners/lava_runner.py b/cros/runners/lava_runner.py
index 8a570dda064f..2e8b1c42bdeb 100755
--- a/cros/runners/lava_runner.py
+++ b/cros/runners/lava_runner.py
@@ -13,7 +13,7 @@ from cros.tests.cros_ec_power import *
from cros.tests.cros_ec_extcon import *
-class LavaTextTestResult(unittest.TestResult):
+class LavaTestResult(unittest.TestResult):
def __init__(self, runner):
unittest.TestResult.__init__(self)
self.runner = runner
@@ -56,7 +56,7 @@ class LavaTestRunner:
self.stream.write(message)
def run(self, test):
- result = LavaTextTestResult(self)
+ result = LavaTestResult(self)
test(result)
result.testsRun
return result
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 02/14] lava_runner: use TextTestRunner
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 01/14] lava_runner: rename to LavaTestResult Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 03/14] lava_runner: use TextTestResult Tzung-Bi Shih
` (12 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
To simplify, eliminate LavaTestRunner but use TextTestRunner directly.
Also change the constructor of LavaTestResult per [1].
As a side-effect result, it starts to show the following summary:
> Ran 18 tests in 0.010s
>
> FAILED (failures=1, errors=3, skipped=10)
[1]: https://docs.python.org/3/library/unittest.html#unittest.TextTestRunner._makeResult
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/runners/lava_runner.py | 29 +++++++----------------------
1 file changed, 7 insertions(+), 22 deletions(-)
diff --git a/cros/runners/lava_runner.py b/cros/runners/lava_runner.py
index 2e8b1c42bdeb..7524f65dcd39 100755
--- a/cros/runners/lava_runner.py
+++ b/cros/runners/lava_runner.py
@@ -14,57 +14,42 @@ from cros.tests.cros_ec_extcon import *
class LavaTestResult(unittest.TestResult):
- def __init__(self, runner):
+ def __init__(self, stream, descriptions, verbosity):
unittest.TestResult.__init__(self)
- self.runner = runner
+ self.stream = stream
def addSuccess(self, test):
unittest.TestResult.addSuccess(self, test)
- self.runner.writeUpdate(
+ self.stream.write(
"<LAVA_SIGNAL_TESTCASE TEST_CASE_ID=%s RESULT=pass>\n"
% test.id().rsplit(".")[-1]
)
def addError(self, test, err):
unittest.TestResult.addError(self, test, err)
- self.runner.writeUpdate(
+ self.stream.write(
"<LAVA_SIGNAL_TESTCASE TEST_CASE_ID=%s RESULT=unknown>\n"
% test.id().rsplit(".")[-1]
)
def addFailure(self, test, err):
unittest.TestResult.addFailure(self, test, err)
- self.runner.writeUpdate(
+ self.stream.write(
"<LAVA_SIGNAL_TESTCASE TEST_CASE_ID=%s RESULT=fail>\n"
% test.id().rsplit(".")[-1]
)
def addSkip(self, test, reason):
unittest.TestResult.addSkip(self, test, reason)
- self.runner.writeUpdate(
+ self.stream.write(
"<LAVA_SIGNAL_TESTCASE TEST_CASE_ID=%s RESULT=skip>\n"
% test.id().rsplit(".")[-1]
)
-class LavaTestRunner:
- def __init__(self, stream=sys.stderr, verbosity=0):
- self.stream = stream
- self.verbosity = verbosity
-
- def writeUpdate(self, message):
- self.stream.write(message)
-
- def run(self, test):
- result = LavaTestResult(self)
- test(result)
- result.testsRun
- return result
-
-
if __name__ == "__main__":
unittest.main(
- testRunner=LavaTestRunner(),
+ testRunner=unittest.TextTestRunner(resultclass=LavaTestResult),
# these make sure that some options that are not applicable
# remain hidden from the help menu.
failfast=False,
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 03/14] lava_runner: use TextTestResult
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 01/14] lava_runner: rename to LavaTestResult Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 02/14] lava_runner: use TextTestRunner Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 04/14] lava_runner: respect to verbosity flag from unittest framework Tzung-Bi Shih
` (11 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
To simplify, make LavaTestResult is-a TextTestResult.
As a side-effect result, it prints errors and fails by the end of test
running. See [1].
[1]: https://github.com/python/cpython/blob/main/Lib/unittest/runner.py#L139
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/runners/lava_runner.py | 44 +++++++++++++++++--------------------
1 file changed, 20 insertions(+), 24 deletions(-)
diff --git a/cros/runners/lava_runner.py b/cros/runners/lava_runner.py
index 7524f65dcd39..70f8aa61132e 100755
--- a/cros/runners/lava_runner.py
+++ b/cros/runners/lava_runner.py
@@ -13,38 +13,34 @@ from cros.tests.cros_ec_power import *
from cros.tests.cros_ec_extcon import *
-class LavaTestResult(unittest.TestResult):
- def __init__(self, stream, descriptions, verbosity):
- unittest.TestResult.__init__(self)
- self.stream = stream
+class LavaTestResult(unittest.TextTestResult):
+ def writeLavaSignal(self, test, result):
+ test_case_id = test.id().rsplit(".")[-1]
- def addSuccess(self, test):
- unittest.TestResult.addSuccess(self, test)
- self.stream.write(
- "<LAVA_SIGNAL_TESTCASE TEST_CASE_ID=%s RESULT=pass>\n"
- % test.id().rsplit(".")[-1]
+ # LAVA signal must be start-of-line. Print a newline if verbosity >= 1.
+ if self.showAll or self.dots:
+ self.stream.writeln()
+
+ self.stream.writeln(
+ f"<LAVA_SIGNAL_TESTCASE TEST_CASE_ID={test_case_id} RESULT={result}>"
)
+ self.stream.flush()
+
+ def addSuccess(self, test):
+ super().addSuccess(test)
+ self.writeLavaSignal(test, "pass")
def addError(self, test, err):
- unittest.TestResult.addError(self, test, err)
- self.stream.write(
- "<LAVA_SIGNAL_TESTCASE TEST_CASE_ID=%s RESULT=unknown>\n"
- % test.id().rsplit(".")[-1]
- )
+ super().addError(test, err)
+ self.writeLavaSignal(test, "unknown")
def addFailure(self, test, err):
- unittest.TestResult.addFailure(self, test, err)
- self.stream.write(
- "<LAVA_SIGNAL_TESTCASE TEST_CASE_ID=%s RESULT=fail>\n"
- % test.id().rsplit(".")[-1]
- )
+ super().addFailure(test, err)
+ self.writeLavaSignal(test, "fail")
def addSkip(self, test, reason):
- unittest.TestResult.addSkip(self, test, reason)
- self.stream.write(
- "<LAVA_SIGNAL_TESTCASE TEST_CASE_ID=%s RESULT=skip>\n"
- % test.id().rsplit(".")[-1]
- )
+ super().addSkip(test, reason)
+ self.writeLavaSignal(test, "skip")
if __name__ == "__main__":
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 04/14] lava_runner: respect to verbosity flag from unittest framework
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (2 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 03/14] lava_runner: use TextTestResult Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 05/14] helpers/sysfs: fix NameError Tzung-Bi Shih
` (10 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
testRunner in unittest.main() shouldn't be an instance. Otherwise, any
verbosity flag won't pass into the testRunner.
For example:
1.
m = unittest.main(
testRunner=TextTestRunner(...),
verbosity=0
)
The verbosity won't take effect in the TextTestRunner. Instead, it uses
the default verbosity in TextTestRunner(), i.e., 1.
2.
m = unittest.main(
testRunner=TextTestRunner(...),
)
When invoking `python3 -m ... -q`, it intends to set the verbosity to 0.
However, the testRunner has been instantiated, the flag won't pass to
the testRunner.
Thus, LavaTestRunner is back.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/runners/lava_runner.py | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/cros/runners/lava_runner.py b/cros/runners/lava_runner.py
index 70f8aa61132e..427d41f2eadb 100755
--- a/cros/runners/lava_runner.py
+++ b/cros/runners/lava_runner.py
@@ -1,6 +1,7 @@
#!/usr/bin/env python3
# -*- coding: utf-8 -*-
+import functools
import sys
import unittest
@@ -43,9 +44,15 @@ class LavaTestResult(unittest.TextTestResult):
self.writeLavaSignal(test, "skip")
+class LavaTestRunner(unittest.TextTestRunner):
+ __init__ = functools.partialmethod(
+ unittest.TextTestRunner.__init__,
+ resultclass=LavaTestResult)
+
+
if __name__ == "__main__":
unittest.main(
- testRunner=unittest.TextTestRunner(resultclass=LavaTestResult),
+ testRunner=LavaTestRunner,
# these make sure that some options that are not applicable
# remain hidden from the help menu.
failfast=False,
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 05/14] helpers/sysfs: fix NameError
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (3 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 04/14] lava_runner: respect to verbosity flag from unittest framework Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 06/14] helpers/mcu: fix ResourceWarning on /dev/cros_ec Tzung-Bi Shih
` (9 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
Some errors have been revealed after the runner is-a TextTestRunner.
Fix the following error:
> NameError: name 'self' is not defined
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/helpers/sysfs.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cros/helpers/sysfs.py b/cros/helpers/sysfs.py
index f3abfabced24..1ccc0cd2ba24 100755
--- a/cros/helpers/sysfs.py
+++ b/cros/helpers/sysfs.py
@@ -33,6 +33,6 @@ def sysfs_check_attributes_exists(s, path, name, files, check_devtype):
for filename in files:
s.assertEqual(os.path.exists(path + "/" + devname + "/" + filename), 1)
except IOError as e:
- self.skipTest("Exception occured: {0}, skipping".format(e.strerror))
+ s.skipTest("Exception occured: {0}, skipping".format(e.strerror))
if match == 0:
s.skipTest("No " + name + " found, skipping")
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 06/14] helpers/mcu: fix ResourceWarning on /dev/cros_ec
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (4 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 05/14] helpers/sysfs: fix NameError Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 07/14] cros_ec_pwm: fix ResourceWarning on /sys/kernel/debug/pwm Tzung-Bi Shih
` (8 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
Some errors have been revealed after the runner is-a TextTestRunner.
Fix the following warning:
> ResourceWarning: unclosed file <_io.TextIOWrapper name='/dev/cros_ec'
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/helpers/mcu.py | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/cros/helpers/mcu.py b/cros/helpers/mcu.py
index e1f09f39af89..80e3328cdf08 100644
--- a/cros/helpers/mcu.py
+++ b/cros/helpers/mcu.py
@@ -112,8 +112,6 @@ def is_feature_supported(feature):
global ECFEATURES
if ECFEATURES == -1:
- fd = open("/dev/cros_ec", "r")
-
param = ec_params_get_features()
response = ec_response_get_features()
@@ -124,11 +122,10 @@ def is_feature_supported(feature):
cmd.outsize = sizeof(response)
memmove(addressof(cmd.data), addressof(param), cmd.outsize)
- fcntl.ioctl(fd, EC_DEV_IOCXCMD, cmd)
+ with open("/dev/cros_ec", "r") as fh:
+ fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
memmove(addressof(response), addressof(cmd.data), cmd.outsize)
- fd.close()
-
if cmd.result == 0:
ECFEATURES = response.out_data
else:
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 07/14] cros_ec_pwm: fix ResourceWarning on /sys/kernel/debug/pwm
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (5 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 06/14] helpers/mcu: fix ResourceWarning on /dev/cros_ec Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 08/14] treewide: use context manager on file objects Tzung-Bi Shih
` (7 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
Some errors have been revealed after the runner is-a TextTestRunner.
Fix the following warning:
> ResourceWarning: unclosed file <_io.TextIOWrapper
name='/sys/kernel/debug/pwm'
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/tests/cros_ec_pwm.py | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/cros/tests/cros_ec_pwm.py b/cros/tests/cros_ec_pwm.py
index bcc9fbcea1d5..ba429a327db1 100644
--- a/cros/tests/cros_ec_pwm.py
+++ b/cros/tests/cros_ec_pwm.py
@@ -37,16 +37,14 @@ class TestCrosECPWM(unittest.TestCase):
fd = open("/sys/class/backlight/backlight/brightness", "w")
fd.write(str(brightness))
fd.close()
- fd = open("/sys/kernel/debug/pwm", "r")
- line = fd.readline()
- while line:
- if "backlight" in line:
- start = line.find("duty") + 6
- self.assertNotEqual(start, 5)
- end = start + line[start:].find(" ")
- self.assertNotEqual(start, end)
- duty = int(line[start:end])
- self.assertNotEqual(duty, 0)
- break
- line = fd.readline()
- fd.close()
+
+ with open("/sys/kernel/debug/pwm", "r") as fh:
+ for line in fh:
+ if "backlight" in line:
+ start = line.find("duty") + 6
+ self.assertNotEqual(start, 5)
+ end = start + line[start:].find(" ")
+ self.assertNotEqual(start, end)
+ duty = int(line[start:end])
+ self.assertNotEqual(duty, 0)
+ break
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 08/14] treewide: use context manager on file objects
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (6 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 07/14] cros_ec_pwm: fix ResourceWarning on /sys/kernel/debug/pwm Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 09/14] cros_ec_pwm: use RE to search EC backlight PWM Tzung-Bi Shih
` (6 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
Use context manager on file objects so that the resource is correctly
released even if an exception happens before it calls close().
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/helpers/kernel.py | 5 ++---
cros/helpers/mcu.py | 17 ++++++-----------
cros/helpers/sysfs.py | 10 ++++------
cros/tests/cros_ec_accel.py | 5 ++---
cros/tests/cros_ec_pwm.py | 10 ++++------
cros/tests/cros_ec_rtc.py | 5 ++---
6 files changed, 20 insertions(+), 32 deletions(-)
diff --git a/cros/helpers/kernel.py b/cros/helpers/kernel.py
index db0c69f01683..9e4d769c0531 100644
--- a/cros/helpers/kernel.py
+++ b/cros/helpers/kernel.py
@@ -14,9 +14,8 @@ def current_kernel_version():
""" Returns the current kernel version as an integer you can
compare.
"""
- fd = open("/proc/version", "r")
- current = fd.read().split()[2].split("-")[0].split(".")
- fd.close()
+ with open("/proc/version", "r") as fh:
+ current = fh.read().split()[2].split("-")[0].split(".")
return version_to_int(int(current[0]), int(current[1]), int(current[2]))
diff --git a/cros/helpers/mcu.py b/cros/helpers/mcu.py
index 80e3328cdf08..27ce9ad5cc58 100644
--- a/cros/helpers/mcu.py
+++ b/cros/helpers/mcu.py
@@ -150,7 +150,6 @@ def mcu_hello(s, name):
""" Checks basic comunication with MCU. """
if not os.path.exists("/dev/" + name):
s.skipTest("MCU " + name + " not present, skipping")
- fd = open("/dev/" + name, "r")
param = ec_params_hello()
param.in_data = 0xA0B0C0D0 # magic number that the EC expects on HELLO
@@ -163,19 +162,16 @@ def mcu_hello(s, name):
cmd.outsize = sizeof(response)
memmove(addressof(cmd.data), addressof(param), cmd.insize)
- fcntl.ioctl(fd, EC_DEV_IOCXCMD, cmd)
+ with open("/dev/" + name, "r") as fh:
+ fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
memmove(addressof(response), addressof(cmd.data), cmd.outsize)
- fd.close()
-
s.assertEqual(cmd.result, 0)
# magic number that the EC answers on HELLO
s.assertEqual(response.out_data, 0xA1B2C3D4)
def mcu_get_version(name):
if os.path.exists("/dev/" + name):
- fd = open("/dev/" + name, "r")
-
response = ec_response_get_version()
cmd = cros_ec_command()
@@ -184,25 +180,24 @@ def mcu_get_version(name):
cmd.insize = sizeof(response)
cmd.outsize = 0
- fcntl.ioctl(fd, EC_DEV_IOCXCMD, cmd)
+ with open("/dev/" + name, "r") as fh:
+ fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
memmove(addressof(response), addressof(cmd.data), cmd.insize)
- fd.close()
if cmd.result == 0:
return response
def mcu_reboot(name):
- fd = open("/dev/" + name, "r")
cmd = cros_ec_command()
cmd.version = 0
cmd.command = EC_CMD_REBOOT
cmd.insize = 0
cmd.outsize = 0
try:
- fcntl.ioctl(fd, EC_DEV_IOCXCMD, cmd)
+ with open("/dev/" + name, "r") as fh:
+ fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
except IOError:
pass
- fd.close()
def check_mcu_reboot_rw(s, name):
if not os.path.exists("/dev/" + name):
diff --git a/cros/helpers/sysfs.py b/cros/helpers/sysfs.py
index 1ccc0cd2ba24..80653fd13431 100755
--- a/cros/helpers/sysfs.py
+++ b/cros/helpers/sysfs.py
@@ -6,9 +6,8 @@ import os
def read_file(name):
""" Returns the content of the file named 'name'."""
- fd = open(name, "r")
- contents = fd.read()
- fd.close()
+ with open(name, "r") as fh:
+ contents = fh.read()
return contents
@@ -21,9 +20,8 @@ def sysfs_check_attributes_exists(s, path, name, files, check_devtype):
try:
for devname in os.listdir(path):
if check_devtype:
- fd = open(path + "/" + devname + "/name", "r")
- devtype = fd.read()
- fd.close()
+ with open(path + "/" + devname + "/name", "r") as fh:
+ devtype = fh.read()
if not devtype.startswith(name):
continue
else:
diff --git a/cros/tests/cros_ec_accel.py b/cros/tests/cros_ec_accel.py
index 8861eb0e1c2c..b76d2a6fbb4d 100755
--- a/cros/tests/cros_ec_accel.py
+++ b/cros/tests/cros_ec_accel.py
@@ -57,8 +57,8 @@ class TestCrosECAccel(unittest.TestCase):
try:
for devname in os.listdir("/sys/bus/iio/devices"):
base_path = "/sys/bus/iio/devices/" + devname + "/"
- fd = open(base_path + "name", "r")
- devtype = fd.read()
+ with open(base_path + "name", "r") as fh:
+ devtype = fh.read()
if devtype.startswith("cros-ec-accel"):
location = read_file(base_path + "location")
accel_scale = float(read_file(base_path + "scale"))
@@ -73,7 +73,6 @@ class TestCrosECAccel(unittest.TestCase):
mag = math.sqrt(mag)
self.assertTrue(abs(mag - exp) <= err)
match += 1
- fd.close()
except IOError as e:
self.skipTest("Exception occured: {0}, skipping".format(e.strerror))
if match == 0:
diff --git a/cros/tests/cros_ec_pwm.py b/cros/tests/cros_ec_pwm.py
index ba429a327db1..2b7b4a3418b1 100644
--- a/cros/tests/cros_ec_pwm.py
+++ b/cros/tests/cros_ec_pwm.py
@@ -31,12 +31,10 @@ class TestCrosECPWM(unittest.TestCase):
fd.close()
if not is_ec_pwm:
self.skipTest("No EC backlight pwm found, skipping")
- fd = open("/sys/class/backlight/backlight/max_brightness", "r")
- brightness = int(int(fd.read()) / 2)
- fd.close()
- fd = open("/sys/class/backlight/backlight/brightness", "w")
- fd.write(str(brightness))
- fd.close()
+ with open("/sys/class/backlight/backlight/max_brightness", "r") as fh:
+ brightness = int(int(fh.read()) / 2)
+ with open("/sys/class/backlight/backlight/brightness", "w") as fh:
+ fh.write(str(brightness))
with open("/sys/kernel/debug/pwm", "r") as fh:
for line in fh:
diff --git a/cros/tests/cros_ec_rtc.py b/cros/tests/cros_ec_rtc.py
index 9f497d2fcb7e..7afed70f0429 100755
--- a/cros/tests/cros_ec_rtc.py
+++ b/cros/tests/cros_ec_rtc.py
@@ -14,9 +14,8 @@ class TestCrosECRTC(unittest.TestCase):
match = 0
try:
for devname in os.listdir("/sys/class/rtc"):
- fd = open("/sys/class/rtc/" + devname + "/name", "r")
- devtype = fd.read()
- fd.close()
+ with open("/sys/class/rtc/" + devname + "/name", "r") as fh:
+ devtype = fh.read()
if devtype.startswith("cros-ec-rtc"):
files = [
"date",
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 09/14] cros_ec_pwm: use RE to search EC backlight PWM
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (7 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 08/14] treewide: use context manager on file objects Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 10/14] helpers/mcu: fix packet too long error Tzung-Bi Shih
` (5 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
Use RE to search pattern instead of reading the file line by line.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/tests/cros_ec_pwm.py | 24 +++++++-----------------
1 file changed, 7 insertions(+), 17 deletions(-)
diff --git a/cros/tests/cros_ec_pwm.py b/cros/tests/cros_ec_pwm.py
index 2b7b4a3418b1..7158463b115f 100644
--- a/cros/tests/cros_ec_pwm.py
+++ b/cros/tests/cros_ec_pwm.py
@@ -2,6 +2,7 @@
# -*- coding: utf-8 -*-
from cros.helpers.sysfs import *
+import re
import unittest
@@ -13,23 +14,12 @@ class TestCrosECPWM(unittest.TestCase):
"""
if not os.path.exists("/sys/class/backlight/backlight/max_brightness"):
self.skipTest("No backlight pwm found, skipping")
- is_ec_pwm = False
- fd = open("/sys/kernel/debug/pwm", "r")
- line = fd.readline()
- while line and not is_ec_pwm:
- if line[0] != " " and ":ec-pwm" in line:
- line = fd.readline()
- while line:
- if line[0] == "\n":
- is_ec_pwm = False
- break
- if "backlight" in line:
- is_ec_pwm = True
- break
- line = fd.readline()
- line = fd.readline()
- fd.close()
- if not is_ec_pwm:
+ with open("/sys/kernel/debug/pwm", "r") as fh:
+ pwm = fh.read()
+ for s in pwm.split('\n\n'):
+ if re.match(r'.*:ec-pwm.*backlight', s, re.DOTALL):
+ break
+ else:
self.skipTest("No EC backlight pwm found, skipping")
with open("/sys/class/backlight/backlight/max_brightness", "r") as fh:
brightness = int(int(fh.read()) / 2)
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 10/14] helpers/mcu: fix packet too long error
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (8 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 09/14] cros_ec_pwm: use RE to search EC backlight PWM Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 11/14] helpers/mcu: fix EC_CMD_GET_FEATURES usage Tzung-Bi Shih
` (4 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
sizeof(c_ulong) is 4 in some platforms. However, EC_CMD_GET_FEATURES
returns 2 uint32[1].
Fix the following error from test execution:
> Traceback (most recent call last):
> File "...cros/tests/cros_ec_rtc.py", line 12, in test_cros_ec_rtc_abi
> if not is_feature_supported(EC_FEATURE_RTC):
> File "...cros/helpers/mcu.py", line 126, in is_feature_supported
> fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
> OSError: [Errno 90] Message too long
And the following error message from dmesg:
> cros-ec-spi spi2.0: packet too long (8 bytes, expected 4)
Use c_uint64 to explicitly specify the data size.
[1]: https://crrev.com/6bf531fc1c115b24e2148fc4e040081ef354cdf6/include/ec_commands.h#1594
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/helpers/mcu.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cros/helpers/mcu.py b/cros/helpers/mcu.py
index 27ce9ad5cc58..506730e71115 100644
--- a/cros/helpers/mcu.py
+++ b/cros/helpers/mcu.py
@@ -90,11 +90,11 @@ class ec_response_get_version(Structure):
]
class ec_params_get_features(Structure):
- _fields_ = [("in_data", c_ulong)]
+ _fields_ = [("in_data", c_uint64)]
class ec_response_get_features(Structure):
- _fields_ = [("out_data", c_ulong)]
+ _fields_ = [("out_data", c_uint64)]
def EC_FEATURE_MASK_0(event_code):
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 11/14] helpers/mcu: fix EC_CMD_GET_FEATURES usage
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (9 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 10/14] helpers/mcu: fix packet too long error Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 12/14] treewide: remove "r" in open() if reading mode Tzung-Bi Shih
` (3 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
EC_CMD_GET_FEATURES has no parameter. Remove it.
Also, the in and out was reversed. Fix it.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/helpers/mcu.py | 16 +++++-----------
1 file changed, 5 insertions(+), 11 deletions(-)
diff --git a/cros/helpers/mcu.py b/cros/helpers/mcu.py
index 506730e71115..7d197e600f47 100644
--- a/cros/helpers/mcu.py
+++ b/cros/helpers/mcu.py
@@ -89,12 +89,8 @@ class ec_response_get_version(Structure):
("current_image", c_uint),
]
-class ec_params_get_features(Structure):
- _fields_ = [("in_data", c_uint64)]
-
-
class ec_response_get_features(Structure):
- _fields_ = [("out_data", c_uint64)]
+ _fields_ = [("in_data", c_uint64)]
def EC_FEATURE_MASK_0(event_code):
@@ -112,22 +108,20 @@ def is_feature_supported(feature):
global ECFEATURES
if ECFEATURES == -1:
- param = ec_params_get_features()
response = ec_response_get_features()
cmd = cros_ec_command()
cmd.version = 0
cmd.command = EC_CMD_GET_FEATURES
- cmd.insize = sizeof(param)
- cmd.outsize = sizeof(response)
+ cmd.insize = sizeof(response)
+ cmd.outsize = 0
- memmove(addressof(cmd.data), addressof(param), cmd.outsize)
with open("/dev/cros_ec", "r") as fh:
fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
- memmove(addressof(response), addressof(cmd.data), cmd.outsize)
+ memmove(addressof(response), addressof(cmd.data), cmd.insize)
if cmd.result == 0:
- ECFEATURES = response.out_data
+ ECFEATURES = response.in_data
else:
return False
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 12/14] treewide: remove "r" in open() if reading mode
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (10 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 11/14] helpers/mcu: fix EC_CMD_GET_FEATURES usage Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 13/14] treewide: don't use "+" operator for constructing paths Tzung-Bi Shih
` (2 subsequent siblings)
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
open()'s default mode is "r". To simplify, remove them if reading
mode.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/helpers/kernel.py | 2 +-
cros/helpers/mcu.py | 8 ++++----
cros/helpers/sysfs.py | 4 ++--
cros/tests/cros_ec_accel.py | 2 +-
cros/tests/cros_ec_pwm.py | 6 +++---
cros/tests/cros_ec_rtc.py | 2 +-
setup.py | 2 +-
7 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/cros/helpers/kernel.py b/cros/helpers/kernel.py
index 9e4d769c0531..c319f99a3143 100644
--- a/cros/helpers/kernel.py
+++ b/cros/helpers/kernel.py
@@ -14,7 +14,7 @@ def current_kernel_version():
""" Returns the current kernel version as an integer you can
compare.
"""
- with open("/proc/version", "r") as fh:
+ with open("/proc/version") as fh:
current = fh.read().split()[2].split("-")[0].split(".")
return version_to_int(int(current[0]), int(current[1]), int(current[2]))
diff --git a/cros/helpers/mcu.py b/cros/helpers/mcu.py
index 7d197e600f47..6c12affd4073 100644
--- a/cros/helpers/mcu.py
+++ b/cros/helpers/mcu.py
@@ -116,7 +116,7 @@ def is_feature_supported(feature):
cmd.insize = sizeof(response)
cmd.outsize = 0
- with open("/dev/cros_ec", "r") as fh:
+ with open("/dev/cros_ec") as fh:
fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
memmove(addressof(response), addressof(cmd.data), cmd.insize)
@@ -156,7 +156,7 @@ def mcu_hello(s, name):
cmd.outsize = sizeof(response)
memmove(addressof(cmd.data), addressof(param), cmd.insize)
- with open("/dev/" + name, "r") as fh:
+ with open("/dev/" + name) as fh:
fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
memmove(addressof(response), addressof(cmd.data), cmd.outsize)
@@ -174,7 +174,7 @@ def mcu_get_version(name):
cmd.insize = sizeof(response)
cmd.outsize = 0
- with open("/dev/" + name, "r") as fh:
+ with open("/dev/" + name) as fh:
fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
memmove(addressof(response), addressof(cmd.data), cmd.insize)
@@ -188,7 +188,7 @@ def mcu_reboot(name):
cmd.insize = 0
cmd.outsize = 0
try:
- with open("/dev/" + name, "r") as fh:
+ with open("/dev/" + name) as fh:
fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
except IOError:
pass
diff --git a/cros/helpers/sysfs.py b/cros/helpers/sysfs.py
index 80653fd13431..4d3b6735ef45 100755
--- a/cros/helpers/sysfs.py
+++ b/cros/helpers/sysfs.py
@@ -6,7 +6,7 @@ import os
def read_file(name):
""" Returns the content of the file named 'name'."""
- with open(name, "r") as fh:
+ with open(name) as fh:
contents = fh.read()
return contents
@@ -20,7 +20,7 @@ def sysfs_check_attributes_exists(s, path, name, files, check_devtype):
try:
for devname in os.listdir(path):
if check_devtype:
- with open(path + "/" + devname + "/name", "r") as fh:
+ with open(path + "/" + devname + "/name") as fh:
devtype = fh.read()
if not devtype.startswith(name):
continue
diff --git a/cros/tests/cros_ec_accel.py b/cros/tests/cros_ec_accel.py
index b76d2a6fbb4d..3d6a54acac16 100755
--- a/cros/tests/cros_ec_accel.py
+++ b/cros/tests/cros_ec_accel.py
@@ -57,7 +57,7 @@ class TestCrosECAccel(unittest.TestCase):
try:
for devname in os.listdir("/sys/bus/iio/devices"):
base_path = "/sys/bus/iio/devices/" + devname + "/"
- with open(base_path + "name", "r") as fh:
+ with open(base_path + "name") as fh:
devtype = fh.read()
if devtype.startswith("cros-ec-accel"):
location = read_file(base_path + "location")
diff --git a/cros/tests/cros_ec_pwm.py b/cros/tests/cros_ec_pwm.py
index 7158463b115f..6be5c292ffa2 100644
--- a/cros/tests/cros_ec_pwm.py
+++ b/cros/tests/cros_ec_pwm.py
@@ -14,19 +14,19 @@ class TestCrosECPWM(unittest.TestCase):
"""
if not os.path.exists("/sys/class/backlight/backlight/max_brightness"):
self.skipTest("No backlight pwm found, skipping")
- with open("/sys/kernel/debug/pwm", "r") as fh:
+ with open("/sys/kernel/debug/pwm") as fh:
pwm = fh.read()
for s in pwm.split('\n\n'):
if re.match(r'.*:ec-pwm.*backlight', s, re.DOTALL):
break
else:
self.skipTest("No EC backlight pwm found, skipping")
- with open("/sys/class/backlight/backlight/max_brightness", "r") as fh:
+ with open("/sys/class/backlight/backlight/max_brightness") as fh:
brightness = int(int(fh.read()) / 2)
with open("/sys/class/backlight/backlight/brightness", "w") as fh:
fh.write(str(brightness))
- with open("/sys/kernel/debug/pwm", "r") as fh:
+ with open("/sys/kernel/debug/pwm") as fh:
for line in fh:
if "backlight" in line:
start = line.find("duty") + 6
diff --git a/cros/tests/cros_ec_rtc.py b/cros/tests/cros_ec_rtc.py
index 7afed70f0429..e59fa383659d 100755
--- a/cros/tests/cros_ec_rtc.py
+++ b/cros/tests/cros_ec_rtc.py
@@ -14,7 +14,7 @@ class TestCrosECRTC(unittest.TestCase):
match = 0
try:
for devname in os.listdir("/sys/class/rtc"):
- with open("/sys/class/rtc/" + devname + "/name", "r") as fh:
+ with open("/sys/class/rtc/" + devname + "/name") as fh:
devtype = fh.read()
if devtype.startswith("cros-ec-rtc"):
files = [
diff --git a/setup.py b/setup.py
index 92e4ed794245..958aaa89e143 100644
--- a/setup.py
+++ b/setup.py
@@ -3,7 +3,7 @@
import setuptools
-with open("README.md", "r") as fh:
+with open("README.md") as fh:
long_description = fh.read()
setuptools.setup(
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 13/14] treewide: don't use "+" operator for constructing paths
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (11 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 12/14] treewide: remove "r" in open() if reading mode Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 14/14] treewide: don't use "+" operator for concatenating strings Tzung-Bi Shih
2023-03-13 11:53 ` [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Ricardo Cañuelo
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
Don't use "+" operator for constructing paths. Use os.path.join()
instead.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/helpers/mcu.py | 19 ++++++++++++-------
cros/helpers/sysfs.py | 6 ++++--
cros/tests/cros_ec_accel.py | 10 +++++-----
cros/tests/cros_ec_extcon.py | 11 ++++++-----
cros/tests/cros_ec_rtc.py | 5 +++--
5 files changed, 30 insertions(+), 21 deletions(-)
diff --git a/cros/helpers/mcu.py b/cros/helpers/mcu.py
index 6c12affd4073..e2d2db0ef1ed 100644
--- a/cros/helpers/mcu.py
+++ b/cros/helpers/mcu.py
@@ -132,7 +132,8 @@ def check_mcu_abi(s, name):
""" Checks that the MCU character device exists in /dev and then verifies
the standard MCU ABI in /sys/class/chromeos.
"""
- if not os.path.exists("/dev/" + name):
+ dev = os.path.join("/dev", name)
+ if not os.path.exists(dev):
s.skipTest("MCU " + name + " not supported, skipping")
files = ["flashinfo", "reboot", "version"]
sysfs_check_attributes_exists(
@@ -142,7 +143,8 @@ def check_mcu_abi(s, name):
def mcu_hello(s, name):
""" Checks basic comunication with MCU. """
- if not os.path.exists("/dev/" + name):
+ dev = os.path.join("/dev", name)
+ if not os.path.exists(dev):
s.skipTest("MCU " + name + " not present, skipping")
param = ec_params_hello()
param.in_data = 0xA0B0C0D0 # magic number that the EC expects on HELLO
@@ -156,7 +158,7 @@ def mcu_hello(s, name):
cmd.outsize = sizeof(response)
memmove(addressof(cmd.data), addressof(param), cmd.insize)
- with open("/dev/" + name) as fh:
+ with open(dev) as fh:
fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
memmove(addressof(response), addressof(cmd.data), cmd.outsize)
@@ -165,7 +167,8 @@ def mcu_hello(s, name):
s.assertEqual(response.out_data, 0xA1B2C3D4)
def mcu_get_version(name):
- if os.path.exists("/dev/" + name):
+ dev = os.path.join("/dev", name)
+ if os.path.exists(dev):
response = ec_response_get_version()
cmd = cros_ec_command()
@@ -174,7 +177,7 @@ def mcu_get_version(name):
cmd.insize = sizeof(response)
cmd.outsize = 0
- with open("/dev/" + name) as fh:
+ with open(dev) as fh:
fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
memmove(addressof(response), addressof(cmd.data), cmd.insize)
@@ -188,13 +191,15 @@ def mcu_reboot(name):
cmd.insize = 0
cmd.outsize = 0
try:
- with open("/dev/" + name) as fh:
+ dev = os.path.join("/dev", name)
+ with open(dev) as fh:
fcntl.ioctl(fh, EC_DEV_IOCXCMD, cmd)
except IOError:
pass
def check_mcu_reboot_rw(s, name):
- if not os.path.exists("/dev/" + name):
+ dev = os.path.join("/dev", name)
+ if not os.path.exists(dev):
s.skipTest("cros_fp not present, skipping")
mcu_reboot(name)
response = mcu_get_version(name)
diff --git a/cros/helpers/sysfs.py b/cros/helpers/sysfs.py
index 4d3b6735ef45..882bc4d14243 100755
--- a/cros/helpers/sysfs.py
+++ b/cros/helpers/sysfs.py
@@ -20,7 +20,8 @@ def sysfs_check_attributes_exists(s, path, name, files, check_devtype):
try:
for devname in os.listdir(path):
if check_devtype:
- with open(path + "/" + devname + "/name") as fh:
+ p = os.path.join(path, devname, "name")
+ with open(p) as fh:
devtype = fh.read()
if not devtype.startswith(name):
continue
@@ -29,7 +30,8 @@ def sysfs_check_attributes_exists(s, path, name, files, check_devtype):
continue
match += 1
for filename in files:
- s.assertEqual(os.path.exists(path + "/" + devname + "/" + filename), 1)
+ p = os.path.join(path, devname, filename)
+ s.assertEqual(os.path.exists(p), 1)
except IOError as e:
s.skipTest("Exception occured: {0}, skipping".format(e.strerror))
if match == 0:
diff --git a/cros/tests/cros_ec_accel.py b/cros/tests/cros_ec_accel.py
index 3d6a54acac16..15a92b7dd35d 100755
--- a/cros/tests/cros_ec_accel.py
+++ b/cros/tests/cros_ec_accel.py
@@ -56,17 +56,17 @@ class TestCrosECAccel(unittest.TestCase):
match = 0
try:
for devname in os.listdir("/sys/bus/iio/devices"):
- base_path = "/sys/bus/iio/devices/" + devname + "/"
- with open(base_path + "name") as fh:
+ base_path = os.path.join("/sys/bus/iio/devices", devname)
+ with open(os.path.join(base_path, "name")) as fh:
devtype = fh.read()
if devtype.startswith("cros-ec-accel"):
- location = read_file(base_path + "location")
- accel_scale = float(read_file(base_path + "scale"))
+ location = read_file(os.path.join(base_path, "location"))
+ accel_scale = float(read_file(os.path.join(base_path, "scale")))
exp = ACCEL_1G_IN_MS2
err = exp * ACCEL_MAG_VALID_OFFSET
mag = 0
for axis in ["x", "y", "z"]:
- axis_path = base_path + "in_accel_" + axis + "_raw"
+ axis_path = os.path.join(base_path, "in_accel_" + axis + "_raw")
value = int(read_file(axis_path))
value *= accel_scale
mag += value * value
diff --git a/cros/tests/cros_ec_extcon.py b/cros/tests/cros_ec_extcon.py
index b2864b52f4c0..8c496b868bb3 100755
--- a/cros/tests/cros_ec_extcon.py
+++ b/cros/tests/cros_ec_extcon.py
@@ -11,22 +11,23 @@ class TestCrosECextcon(unittest.TestCase):
match = 0
try:
for devname in os.listdir("/sys/class/extcon"):
- devtype = read_file("/sys/class/extcon/" + devname + "/name")
+ d = os.path.join("/sys/class/extcon", devname)
+ devtype = read_file(os.path.join(d, "name"))
if ".spi:ec@0:extcon@" in devtype:
self.assertEqual(
- os.path.exists("/sys/class/extcon/" + devname + "/state"), 1
+ os.path.exists(os.path.join(d, "state")), 1
)
- for cable in os.listdir("/sys/class/extcon/" + devname):
+ for cable in os.listdir(d):
if cable.startswith("cable"):
self.assertEqual(
os.path.exists(
- "/sys/class/extcon/" + devname + "/" + cable + "/name"
+ os.path.join(d, cable, "name")
),
1,
)
self.assertEqual(
os.path.exists(
- "/sys/class/extcon/" + devname + "/" + cable + "/state"
+ os.path.join(d, cable, "state")
),
1,
)
diff --git a/cros/tests/cros_ec_rtc.py b/cros/tests/cros_ec_rtc.py
index e59fa383659d..2d1ef3d58d85 100755
--- a/cros/tests/cros_ec_rtc.py
+++ b/cros/tests/cros_ec_rtc.py
@@ -14,7 +14,8 @@ class TestCrosECRTC(unittest.TestCase):
match = 0
try:
for devname in os.listdir("/sys/class/rtc"):
- with open("/sys/class/rtc/" + devname + "/name") as fh:
+ d = os.path.join("/sys/class/rtc", devname)
+ with open(os.path.join(d, "name")) as fh:
devtype = fh.read()
if devtype.startswith("cros-ec-rtc"):
files = [
@@ -28,7 +29,7 @@ class TestCrosECRTC(unittest.TestCase):
match += 1
for filename in files:
self.assertEqual(
- os.path.exists("/sys/class/rtc/" + devname + "/" + filename), 1
+ os.path.exists(os.path.join(d, filename)), 1
)
except IOError as e:
self.skipTest("Exception occured: {0}, skipping".format(e.strerror))
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 14/14] treewide: don't use "+" operator for concatenating strings
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (12 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 13/14] treewide: don't use "+" operator for constructing paths Tzung-Bi Shih
@ 2023-03-13 9:44 ` Tzung-Bi Shih
2023-03-13 11:53 ` [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Ricardo Cañuelo
14 siblings, 0 replies; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-13 9:44 UTC (permalink / raw)
To: bleung, groeck
Cc: chrome-platform, tzungbi, guillaume.tucker, denys.f, ricardo.canuelo
Don't use "+" operator for concatenating strings. Use f-string instead.
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
cros/helpers/mcu.py | 4 ++--
cros/tests/cros_ec_accel.py | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cros/helpers/mcu.py b/cros/helpers/mcu.py
index e2d2db0ef1ed..d756f25a9c6e 100644
--- a/cros/helpers/mcu.py
+++ b/cros/helpers/mcu.py
@@ -134,7 +134,7 @@ def check_mcu_abi(s, name):
"""
dev = os.path.join("/dev", name)
if not os.path.exists(dev):
- s.skipTest("MCU " + name + " not supported, skipping")
+ s.skipTest(f"MCU {name} not supported, skipping")
files = ["flashinfo", "reboot", "version"]
sysfs_check_attributes_exists(
s, "/sys/class/chromeos/", name, files, False
@@ -145,7 +145,7 @@ def mcu_hello(s, name):
""" Checks basic comunication with MCU. """
dev = os.path.join("/dev", name)
if not os.path.exists(dev):
- s.skipTest("MCU " + name + " not present, skipping")
+ s.skipTest(f"MCU {name} not present, skipping")
param = ec_params_hello()
param.in_data = 0xA0B0C0D0 # magic number that the EC expects on HELLO
diff --git a/cros/tests/cros_ec_accel.py b/cros/tests/cros_ec_accel.py
index 15a92b7dd35d..22d7affe3632 100755
--- a/cros/tests/cros_ec_accel.py
+++ b/cros/tests/cros_ec_accel.py
@@ -66,7 +66,7 @@ class TestCrosECAccel(unittest.TestCase):
err = exp * ACCEL_MAG_VALID_OFFSET
mag = 0
for axis in ["x", "y", "z"]:
- axis_path = os.path.join(base_path, "in_accel_" + axis + "_raw")
+ axis_path = os.path.join(base_path, f"in_accel_{axis}_raw")
value = int(read_file(axis_path))
value *= accel_scale
mag += value * value
--
2.40.0.rc1.284.g88254d51c5-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
` (13 preceding siblings ...)
2023-03-13 9:44 ` [PATCH 14/14] treewide: don't use "+" operator for concatenating strings Tzung-Bi Shih
@ 2023-03-13 11:53 ` Ricardo Cañuelo
2023-03-14 3:09 ` Tzung-Bi Shih
14 siblings, 1 reply; 18+ messages in thread
From: Ricardo Cañuelo @ 2023-03-13 11:53 UTC (permalink / raw)
To: Tzung-Bi Shih, bleung, groeck; +Cc: chrome-platform, guillaume.tucker, denys.f
Hi all,
On 13/3/23 10:44, Tzung-Bi Shih wrote:> The series changes cros-ec-tests[1].
>
> It applys some clean-ups and fixes obvious errors.
>
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/cros-ec-tests.git/
FWIW, there's also this fork [1], that's a bit ahead of [2] and
it's the one we're currently using for KernelCI [3], since Enric
is not maintaining the old one anymore.
Benson, is the cros-ec repo at kernel.org still alive and
maintained? If so we'll sync it to [1] after this patch lands and
point KernelCI to it.
Cheers,
Ricardo
[1]: https://github.com/kernelci/cros-ec-tests
[2]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/cros-ec-tests.git/
[3]: https://github.com/kernelci/kernelci-core/pull/1722
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups
2023-03-13 11:53 ` [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Ricardo Cañuelo
@ 2023-03-14 3:09 ` Tzung-Bi Shih
2023-03-14 6:47 ` Guillaume Charles Tucker
0 siblings, 1 reply; 18+ messages in thread
From: Tzung-Bi Shih @ 2023-03-14 3:09 UTC (permalink / raw)
To: Ricardo Cañuelo
Cc: bleung, groeck, chrome-platform, guillaume.tucker, denys.f
Hi Ricardo,
On Mon, Mar 13, 2023 at 12:53:01PM +0100, Ricardo Cañuelo wrote:
> On 13/3/23 10:44, Tzung-Bi Shih wrote:> The series changes cros-ec-tests[1].
> >
> > It applys some clean-ups and fixes obvious errors.
> >
> > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/cros-ec-tests.git/
>
> FWIW, there's also this fork [1], that's a bit ahead of [2] and
> it's the one we're currently using for KernelCI [3], since Enric
> is not maintaining the old one anymore.
My original plan was refactoring and cleaning [2] first. And then porting [4]
to the latest code base.
[4]: https://github.com/kernelci/cros-ec-tests/commit/f6c0dbf63842d5751000c5527808aca38354db55
> Benson, is the cros-ec repo at kernel.org still alive and
> maintained? If so we'll sync it to [1] after this patch lands and
> point KernelCI to it.
I have no strong preference for where to host the repo. It would be fine if
we abandoned the cros-ec-tests repo at kernel.org.
I guess it would be easier to start from [1] as [3] already pointed to it. If
it makes sense, I would rebase the series and send to [1].
> [1]: https://github.com/kernelci/cros-ec-tests
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/cros-ec-tests.git/
> [3]: https://github.com/kernelci/kernelci-core/pull/1722
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups
2023-03-14 3:09 ` Tzung-Bi Shih
@ 2023-03-14 6:47 ` Guillaume Charles Tucker
0 siblings, 0 replies; 18+ messages in thread
From: Guillaume Charles Tucker @ 2023-03-14 6:47 UTC (permalink / raw)
To: Tzung-Bi Shih
Cc: Ricardo Cañuelo, bleung, groeck, chrome-platform, denys.f
On Tuesday, March 14, 2023 04:09 CET, Tzung-Bi Shih <tzungbi@kernel.org> wrote:
> Hi Ricardo,
>
> On Mon, Mar 13, 2023 at 12:53:01PM +0100, Ricardo Cañuelo wrote:
> > On 13/3/23 10:44, Tzung-Bi Shih wrote:> The series changes cros-ec-tests[1].
> > >
> > > It applys some clean-ups and fixes obvious errors.
> > >
> > > [1]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/cros-ec-tests.git/
> >
> > FWIW, there's also this fork [1], that's a bit ahead of [2] and
> > it's the one we're currently using for KernelCI [3], since Enric
> > is not maintaining the old one anymore.
>
> My original plan was refactoring and cleaning [2] first. And then porting [4]
> to the latest code base.
>
> [4]: https://github.com/kernelci/cros-ec-tests/commit/f6c0dbf63842d5751000c5527808aca38354db55
>
> > Benson, is the cros-ec repo at kernel.org still alive and
> > maintained? If so we'll sync it to [1] after this patch lands and
> > point KernelCI to it.
>
> I have no strong preference for where to host the repo. It would be fine if
> we abandoned the cros-ec-tests repo at kernel.org.
>
> I guess it would be easier to start from [1] as [3] already pointed to it. If
> it makes sense, I would rebase the series and send to [1].
>
> > [1]: https://github.com/kernelci/cros-ec-tests
> > [2]: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/cros-ec-tests.git/
> > [3]: https://github.com/kernelci/kernelci-core/pull/1722
I believe it would be much better to have this on kernel.org as long as we have a maintainer available.
Guillaume
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2023-03-14 6:47 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-13 9:44 [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 01/14] lava_runner: rename to LavaTestResult Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 02/14] lava_runner: use TextTestRunner Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 03/14] lava_runner: use TextTestResult Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 04/14] lava_runner: respect to verbosity flag from unittest framework Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 05/14] helpers/sysfs: fix NameError Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 06/14] helpers/mcu: fix ResourceWarning on /dev/cros_ec Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 07/14] cros_ec_pwm: fix ResourceWarning on /sys/kernel/debug/pwm Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 08/14] treewide: use context manager on file objects Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 09/14] cros_ec_pwm: use RE to search EC backlight PWM Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 10/14] helpers/mcu: fix packet too long error Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 11/14] helpers/mcu: fix EC_CMD_GET_FEATURES usage Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 12/14] treewide: remove "r" in open() if reading mode Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 13/14] treewide: don't use "+" operator for constructing paths Tzung-Bi Shih
2023-03-13 9:44 ` [PATCH 14/14] treewide: don't use "+" operator for concatenating strings Tzung-Bi Shih
2023-03-13 11:53 ` [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups Ricardo Cañuelo
2023-03-14 3:09 ` Tzung-Bi Shih
2023-03-14 6:47 ` Guillaume Charles Tucker
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).