[kms-test] [PATCH 0/7] Miscellaneous fixes and improvements

[kms-test] [PATCH 0/7] Miscellaneous fixes and improvements

[Laurent Pinchart]

Hello,

This series fixes miscellaneous issues in the kms-test suite ([1]).

Patches 1/7 to 3/7 are direct bug fixes, patches 4/7 and 5/7 improve
handling of CRC reader failures by properly handling exceptions, and
patches 6/7 and 7/7 implement a test runner. Please see individual
patches for details.

[1] https://git.ideasonboard.com/renesas/kms-tests.git

Laurent Pinchart (7):
  tests: brxalloc: Fix test start log message
  tests: modes: Fix access to RuntimeError message text
  kmstest: Fix CRC directory name string
  kmstest: Fix exception handling in CRCReader constructor and
    destructor
  tests: crc: Skip test if CRC support isn't available
  tests: Only call execute() if the test is run directly
  kmstest: Implement test runner when executed directly

 tests/kms-test-allplanes.py      |  4 ++-
 tests/kms-test-brxalloc.py       |  6 ++--
 tests/kms-test-connectors.py     |  3 +-
 tests/kms-test-crc.py            | 12 ++++++--
 tests/kms-test-formats.py        |  4 ++-
 tests/kms-test-legacy-modeset.py |  4 ++-
 tests/kms-test-modes.py          |  6 ++--
 tests/kms-test-modeset.py        |  4 ++-
 tests/kms-test-pageflip.py       |  4 ++-
 tests/kms-test-plane-alpha.py    |  4 ++-
 tests/kms-test-plane-position.py |  4 ++-
 tests/kms-test-plane-zpos.py     |  4 ++-
 tests/kms-test-routing.py        |  3 +-
 tests/kmstest.py                 | 52 +++++++++++++++++++++++++++++---
 14 files changed, 94 insertions(+), 20 deletions(-)


base-commit: 70afa1a51f4ffc4db404514af0b2990b23d56fee
-- 
Regards,

Laurent Pinchart

[kms-test] [PATCH 1/7] tests: brxalloc: Fix test start log message

[Laurent Pinchart]

The test start log message incorrectly prints the same connector twice
instead of the source and destination connectors. Fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/kms-test-brxalloc.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms-test-brxalloc.py b/tests/kms-test-brxalloc.py
index 7170ee6c7708..c80d2e33c846 100755
--- a/tests/kms-test-brxalloc.py
+++ b/tests/kms-test-brxalloc.py
@@ -62,7 +62,7 @@ class BRxAllocTest(kmstest.KMSTest):
             return
 
         self.start(f'Moving {len(planes)} planes from {outputs[0].connector.fullname} '
-                   f'to {outputs[0].connector.fullname}')
+                   f'to {outputs[1].connector.fullname}')
 
         # Set the initial mode for both outputs and wait 5s for the monitors to
         # wake up.
-- 
Regards,

Laurent Pinchart

[kms-test] [PATCH 2/7] tests: modes: Fix access to RuntimeError message text

[Laurent Pinchart]

The RuntimeError class has no message member. Use args[0] instead.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/kms-test-modes.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kms-test-modes.py b/tests/kms-test-modes.py
index c47848904ec4..14463add7802 100755
--- a/tests/kms-test-modes.py
+++ b/tests/kms-test-modes.py
@@ -60,7 +60,7 @@ class ModesTest(kmstest.KMSTest):
                     self.progress(i+1, len(modes))
                     self.test_mode(connector, crtc, modes[i])
                 except RuntimeError as e:
-                    self.fail(e.message)
+                    self.fail(e.args[0])
                     break
             else:
                 self.success()
-- 
Regards,

Laurent Pinchart

[kms-test] [PATCH 3/7] kmstest: Fix CRC directory name string

[Laurent Pinchart]

The CRC directory name string was converted to a Python formatted string
literal incorrectly, with the f prefix within the string instead of just
before it. Fix it.

Fixes: dcbc93a117b0 ("tests: Convert to formatted string literals")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/kmstest.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/kmstest.py b/tests/kmstest.py
index 18432375c1ca..177110ebd894 100755
--- a/tests/kmstest.py
+++ b/tests/kmstest.py
@@ -171,7 +171,7 @@ class CRCReader(object):
 
         # Hardcode the device minor to 0 as the KMSTest constructor opens the
         # default card object.
-        self.dir = os.open('f/sys/kernel/debug/dri/0/crtc-{self.pipe}/crc', 0)
+        self.dir = os.open(f'/sys/kernel/debug/dri/0/crtc-{self.pipe}/crc', 0)
         self.ctrl = os.open('control', os.O_WRONLY, dir_fd = self.dir)
         self.data = -1
 
-- 
Regards,

Laurent Pinchart

[kms-test] [PATCH 4/7] kmstest: Fix exception handling in CRCReader constructor and destructor

[Laurent Pinchart]

If an exception occurs in the CRCReader constructor, the self.dir,
self.ctrl and self.data members may not be set. This causes another
exception to be thrown by the destructor when it tries to access those
members. Fix it by initializing all the members first, and only closing
the dir and ctrl file descriptors if they have been successfully opened.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/kmstest.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tests/kmstest.py b/tests/kmstest.py
index 177110ebd894..960c27f301ce 100755
--- a/tests/kmstest.py
+++ b/tests/kmstest.py
@@ -168,17 +168,21 @@ class CRCReader(object):
 
     def __init__(self, crtc):
         self.pipe = crtc.idx
+        self.ctrl = -1
+        self.dir = -1
+        self.data = -1
 
         # Hardcode the device minor to 0 as the KMSTest constructor opens the
         # default card object.
         self.dir = os.open(f'/sys/kernel/debug/dri/0/crtc-{self.pipe}/crc', 0)
         self.ctrl = os.open('control', os.O_WRONLY, dir_fd = self.dir)
-        self.data = -1
 
     def __del__(self):
         self.stop()
-        os.close(self.ctrl)
-        os.close(self.dir)
+        if self.ctrl != -1:
+            os.close(self.ctrl)
+        if self.dir != -1:
+            os.close(self.dir)
 
     def start(self, source):
         os.write(self.ctrl, source.encode('ascii'))
-- 
Regards,

Laurent Pinchart

[kms-test] [PATCH 5/7] tests: crc: Skip test if CRC support isn't available

[Laurent Pinchart]

If CRC support isn't available (most likely caused by debugfs not being
mounted), the CRCReader constructor will throw an exception that the crc
test doesn't handle. Move construction of the CRCReader earlier and skip
the test if it fails.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/kms-test-crc.py | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/tests/kms-test-crc.py b/tests/kms-test-crc.py
index efb3cf0dbe41..01f9606f2de9 100755
--- a/tests/kms-test-crc.py
+++ b/tests/kms-test-crc.py
@@ -123,6 +123,13 @@ class CRCTest(kmstest.KMSTest):
         for crtc in self.card.crtcs:
             self.start(f'CRC calculation on CRTC {crtc.id}')
 
+            try:
+                crc_reader = kmstest.CRCReader(crtc)
+            except:
+                self.logger.log('Failed to create CRC reader, check that debugfs is mounted')
+                self.skip(f'CRC support not available for CRTC {crtc.id}')
+                continue
+
             # Get the connector and default mode
             try:
                 connector = connectors[crtc];
@@ -198,7 +205,6 @@ class CRCTest(kmstest.KMSTest):
                 # Set the CRC source and acquire 10 CRC values. Discard the
                 # first value, as the device is running and the new source
                 # needs one frame to take effect.
-                crc_reader = kmstest.CRCReader(crtc)
                 crc_reader.start(crc_source)
                 crcs = crc_reader.read(10)
                 crc_reader.stop()
-- 
Regards,

Laurent Pinchart

[kms-test] [PATCH 6/7] tests: Only call execute() if the test is run directly

[Laurent Pinchart]

Only run a test automatically if the Python test script is executed
directly. This will allow importing tests as modules in a test runner.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/kms-test-allplanes.py      | 4 +++-
 tests/kms-test-brxalloc.py       | 4 +++-
 tests/kms-test-connectors.py     | 3 ++-
 tests/kms-test-crc.py            | 4 +++-
 tests/kms-test-formats.py        | 4 +++-
 tests/kms-test-legacy-modeset.py | 4 +++-
 tests/kms-test-modes.py          | 4 +++-
 tests/kms-test-modeset.py        | 4 +++-
 tests/kms-test-pageflip.py       | 4 +++-
 tests/kms-test-plane-alpha.py    | 4 +++-
 tests/kms-test-plane-position.py | 4 +++-
 tests/kms-test-plane-zpos.py     | 4 +++-
 tests/kms-test-routing.py        | 3 ++-
 13 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/tests/kms-test-allplanes.py b/tests/kms-test-allplanes.py
index 96ac4f19f538..6a6f7e72ca52 100755
--- a/tests/kms-test-allplanes.py
+++ b/tests/kms-test-allplanes.py
@@ -85,4 +85,6 @@ class AllPlanesTest(kmstest.KMSTest):
 
             self.atomic_crtc_disable(crtc)
 
-AllPlanesTest().execute()
+
+if __name__ == '__main__':
+    AllPlanesTest().execute()
diff --git a/tests/kms-test-brxalloc.py b/tests/kms-test-brxalloc.py
index c80d2e33c846..3ccd6995b449 100755
--- a/tests/kms-test-brxalloc.py
+++ b/tests/kms-test-brxalloc.py
@@ -128,4 +128,6 @@ class BRxAllocTest(kmstest.KMSTest):
         self.atomic_crtc_disable(outputs[0].crtc)
         self.atomic_crtc_disable(outputs[1].crtc)
 
-BRxAllocTest().execute()
+
+if __name__ == '__main__':
+    BRxAllocTest().execute()
diff --git a/tests/kms-test-connectors.py b/tests/kms-test-connectors.py
index d2ca636eb2a0..fdbe8bdf6e97 100755
--- a/tests/kms-test-connectors.py
+++ b/tests/kms-test-connectors.py
@@ -27,4 +27,5 @@ class ConnectorsTest(kmstest.KMSTest):
 
             self.success()
 
-ConnectorsTest().execute()
+if __name__ == '__main__':
+    ConnectorsTest().execute()
diff --git a/tests/kms-test-crc.py b/tests/kms-test-crc.py
index 01f9606f2de9..b1af65d5256d 100755
--- a/tests/kms-test-crc.py
+++ b/tests/kms-test-crc.py
@@ -230,4 +230,6 @@ class CRCTest(kmstest.KMSTest):
 
             self.atomic_crtc_disable(crtc)
 
-CRCTest().execute()
+
+if __name__ == '__main__':
+    CRCTest().execute()
diff --git a/tests/kms-test-formats.py b/tests/kms-test-formats.py
index 5c3bfd441a38..afc5454d0b5d 100755
--- a/tests/kms-test-formats.py
+++ b/tests/kms-test-formats.py
@@ -75,4 +75,6 @@ class FormatsTest(kmstest.KMSTest):
         else:
             self.success()
 
-FormatsTest().execute()
+
+if __name__ == '__main__':
+    FormatsTest().execute()
diff --git a/tests/kms-test-legacy-modeset.py b/tests/kms-test-legacy-modeset.py
index 06eceade19c4..4082093ab985 100755
--- a/tests/kms-test-legacy-modeset.py
+++ b/tests/kms-test-legacy-modeset.py
@@ -59,4 +59,6 @@ class LegacyModeSetTest(kmstest.KMSTest):
 
             self.success()
 
-LegacyModeSetTest().execute()
+
+if __name__ == '__main__':
+    LegacyModeSetTest().execute()
diff --git a/tests/kms-test-modes.py b/tests/kms-test-modes.py
index 14463add7802..d857dd6e6f05 100755
--- a/tests/kms-test-modes.py
+++ b/tests/kms-test-modes.py
@@ -65,4 +65,6 @@ class ModesTest(kmstest.KMSTest):
             else:
                 self.success()
 
-ModesTest().execute()
+
+if __name__ == '__main__':
+    ModesTest().execute()
diff --git a/tests/kms-test-modeset.py b/tests/kms-test-modeset.py
index e8b3d9ac37f9..fd0c4acb91de 100755
--- a/tests/kms-test-modeset.py
+++ b/tests/kms-test-modeset.py
@@ -58,4 +58,6 @@ class ModeSetTest(kmstest.KMSTest):
             else:
                 self.success()
 
-ModeSetTest().execute()
+
+if __name__ == '__main__':
+    ModeSetTest().execute()
diff --git a/tests/kms-test-pageflip.py b/tests/kms-test-pageflip.py
index 14bea0a877a3..1bb1214080ff 100755
--- a/tests/kms-test-pageflip.py
+++ b/tests/kms-test-pageflip.py
@@ -118,4 +118,6 @@ class PageFlipTest(kmstest.KMSTest):
                             f'({self.flips}/{frames} frames in {interval} s)')
             self.success()
 
-PageFlipTest().execute()
+
+if __name__ == '__main__':
+    PageFlipTest().execute()
diff --git a/tests/kms-test-plane-alpha.py b/tests/kms-test-plane-alpha.py
index 8802bfeae40c..894e4ab8baaa 100755
--- a/tests/kms-test-plane-alpha.py
+++ b/tests/kms-test-plane-alpha.py
@@ -94,4 +94,6 @@ class PlaneAlphaTest(kmstest.KMSTest):
 
         self.atomic_crtc_disable(crtc)
 
-PlaneAlphaTest().execute()
+
+if __name__ == '__main__':
+    PlaneAlphaTest().execute()
diff --git a/tests/kms-test-plane-position.py b/tests/kms-test-plane-position.py
index aceb849950a1..5dfcfaaba05e 100755
--- a/tests/kms-test-plane-position.py
+++ b/tests/kms-test-plane-position.py
@@ -123,4 +123,6 @@ class PlanePositionTest(kmstest.KMSTest):
         self.atomic_crtc_disable(crtc)
         self.success()
 
-PlanePositionTest().execute()
+
+if __name__ == '__main__':
+    PlanePositionTest().execute()
diff --git a/tests/kms-test-plane-zpos.py b/tests/kms-test-plane-zpos.py
index 052eea542ec0..d27008823827 100755
--- a/tests/kms-test-plane-zpos.py
+++ b/tests/kms-test-plane-zpos.py
@@ -99,4 +99,6 @@ class PlaneZPosTest(kmstest.KMSTest):
 
         self.atomic_crtc_disable(crtc)
 
-PlaneZPosTest().execute()
+
+if __name__ == '__main__':
+    PlaneZPosTest().execute()
diff --git a/tests/kms-test-routing.py b/tests/kms-test-routing.py
index 68aff9c11cff..ccf61d83488d 100755
--- a/tests/kms-test-routing.py
+++ b/tests/kms-test-routing.py
@@ -141,4 +141,5 @@ class RoutingTest(kmstest.KMSTest):
         return allocated
 
 
-RoutingTest().execute()
+if __name__ == '__main__':
+    RoutingTest().execute()
-- 
Regards,

Laurent Pinchart

[kms-test] [PATCH 7/7] kmstest: Implement test runner when executed directly

[Laurent Pinchart]

Run all tests from the current directory when the kmstest.py script it
executed directly, as opposed to being imported by individual tests.
This simplifies running all tests.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/kmstest.py | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tests/kmstest.py b/tests/kmstest.py
index 960c27f301ce..e84d8fe859f4 100755
--- a/tests/kmstest.py
+++ b/tests/kmstest.py
@@ -334,7 +334,17 @@ class KMSTest(object):
         if use_default_key_handler:
             self.loop.register(sys.stdin, selectors.EVENT_READ, self.__read_key)
 
+    def __enter__(self):
+        return self
+
+    def __exit__(self, *err):
+        self.card = None
+        self.loop.close()
+        self.logger.close()
+
     def __del__(self):
+        self.card = None
+        self.loop.close()
         self.logger.close()
 
     def atomic_crtc_disable(self, crtc, sync=True):
@@ -520,3 +530,33 @@ class KMSTest(object):
         sys.stdout.write(f'\rTesting {self.test_name}: SUCCESS\n')
         sys.stdout.flush()
 
+
+if __name__ == '__main__':
+    import importlib
+    import inspect
+    import os
+
+    files = []
+    for path in os.scandir():
+        if path.is_file() and path.name.startswith('kms-test-') and path.name.endswith('.py'):
+            files.append(path.name)
+
+    files.sort()
+    for file in files:
+        print(f'- {file}')
+        module = importlib.import_module(file[:-3])
+        tests = []
+        for name in dir(module):
+            obj = getattr(module, name)
+            if not isinstance(obj, type):
+                continue
+
+            if 'KMSTest' in [cls.__name__ for cls in inspect.getmro(obj)]:
+                tests.append(obj)
+
+        for test in tests:
+            # Use a context manager to ensure proper cleanup after each test,
+            # otherwise state from one test may leak to the other based on when
+            # objects end up being deleted.
+            with test() as test:
+                test.execute()
-- 
Regards,

Laurent Pinchart

[kms-test] [PATCH 8/7] tests: Don't skip formats and plane-position tests on unconnected connector

[Laurent Pinchart]

The formats and plane-position tests need one connected connector. Don't
skip them upon the first unconnected connector, only when no connected
connector can be found.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/kms-test-formats.py        | 2 +-
 tests/kms-test-plane-position.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/kms-test-formats.py b/tests/kms-test-formats.py
index afc5454d0b5d..a4f4fbfa3071 100755
--- a/tests/kms-test-formats.py
+++ b/tests/kms-test-formats.py
@@ -15,7 +15,7 @@ class FormatsTest(kmstest.KMSTest):
         # Find a CRTC with a connected connector and at least one plane
         for connector in self.output_connectors():
             if not connector.connected():
-                self.skip('unconnected connector')
+                self.logger.log(f'unconnected connector {connector.fullname}')
                 continue
 
             try:
diff --git a/tests/kms-test-plane-position.py b/tests/kms-test-plane-position.py
index 5dfcfaaba05e..c85375adc0d2 100755
--- a/tests/kms-test-plane-position.py
+++ b/tests/kms-test-plane-position.py
@@ -15,7 +15,7 @@ class PlanePositionTest(kmstest.KMSTest):
         # Find a CRTC with a connected connector and at least two planes
         for connector in self.output_connectors():
             if not connector.connected():
-                self.skip('unconnected connector')
+                self.logger.log(f'unconnected connector {connector.fullname}')
                 continue
 
             try:

-- 
Regards,

Laurent Pinchart

[kms-test] [PATCH 9/7] kmstest: Raise clear exception when property is not found

[Laurent Pinchart]

When converting a property value string to an integer, if the property
is not found, an AttributeError exception is raised and reports that
"'NoneType' object has no attribute 'type'". This isn't very clear,
replace it with a RuntimeError with a proper message.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 tests/kmstest.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/kmstest.py b/tests/kmstest.py
index e84d8fe859f4..3e53defc1dae 100755
--- a/tests/kmstest.py
+++ b/tests/kmstest.py
@@ -268,6 +268,8 @@ class AtomicRequest(pykms.AtomicReq):
             if isinstance(v, str):
                 if v.endswith('%'):
                     prop = obj.get_prop(k)
+                    if not prop:
+                        raise RuntimeError(f'Property {k} not supported by object {obj}')
                     if prop.type not in (pykms.PropertyType.Range, pykms.PropertyType.SignedRange):
                         raise RuntimeError(f'Unsupported property type {prop.type} for value {v}')
 
-- 
Regards,

Laurent Pinchart