[PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups

[PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups

[Tzung-Bi Shih]

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

[PATCH 01/14] lava_runner: rename to LavaTestResult

[Tzung-Bi Shih]

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

[PATCH 02/14] lava_runner: use TextTestRunner

[Tzung-Bi Shih]

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

[PATCH 03/14] lava_runner: use TextTestResult

[Tzung-Bi Shih]

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

[PATCH 04/14] lava_runner: respect to verbosity flag from unittest framework

[Tzung-Bi Shih]

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

[PATCH 05/14] helpers/sysfs: fix NameError

[Tzung-Bi Shih]

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

[PATCH 06/14] helpers/mcu: fix ResourceWarning on /dev/cros_ec

[Tzung-Bi Shih]

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

[PATCH 07/14] cros_ec_pwm: fix ResourceWarning on /sys/kernel/debug/pwm

[Tzung-Bi Shih]

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

[PATCH 08/14] treewide: use context manager on file objects

[Tzung-Bi Shih]

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

[PATCH 09/14] cros_ec_pwm: use RE to search EC backlight PWM

[Tzung-Bi Shih]

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

[PATCH 10/14] helpers/mcu: fix packet too long error

[Tzung-Bi Shih]

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

[PATCH 11/14] helpers/mcu: fix EC_CMD_GET_FEATURES usage

[Tzung-Bi Shih]

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

[PATCH 12/14] treewide: remove "r" in open() if reading mode

[Tzung-Bi Shih]

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

[PATCH 13/14] treewide: don't use "+" operator for constructing paths

[Tzung-Bi Shih]

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

[PATCH 14/14] treewide: don't use "+" operator for concatenating strings

[Tzung-Bi Shih]

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

Re: [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups

[Ricardo Cañuelo]

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

Re: [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups

[Tzung-Bi Shih]

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

Re: [PATCH 00/14] cros-ec-tests: fix some exceptions and clean-ups

[Guillaume Charles Tucker]

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