[libcamera-devel] [PATCH v11 0/5] lc-compliance: Refactor using Googletest
Nícolas F. R. A. Prado
nfraprado at collabora.com
Tue Jul 6 14:19:14 CEST 2021
On Tue, Jul 06, 2021 at 12:42:53PM +0200, Jacopo Mondi wrote:
> Hi Nicolas,
>
> On Fri, Jul 02, 2021 at 09:21:09AM -0300, Nícolas F. R. A. Prado wrote:
> > (We're now running lc-compliance on KernelCI, more details below)
> >
> > This is a refactoring of lc-compliance using Googletest as the framework.
> >
> > Patches 1 and 2 are just to simplify the cleanup path from the tests. Patch 3
> > adds an Environment singleton to provide access to the camera from all tests.
> > Patch 4 does the actual refactor. Patch 5 adds two new parameters, --list and
> > --filter.
>
> Now pushed to master, thank you!
Awesome! Thank you all for the reviews in this v11 journey :)
>
> >
> > lc-compliance on KernelCI
> > -------------------------
> >
> > lc-compliance is already being run on KernelCI [1] for mt8173-elm-hana
> > (Chromebook), which has an USB camera. All test runs of lc-compliance so far can
> > be seen on the Tests tab [2] by searching for "lc-compliance". I'm currently
> > looking into adding Rock Pi 4 boards to the lab so that the tests can run on
> > those too.
> >
> > [1] https://linux.kernelci.org/test/job/tip/branch/master/kernel/v5.13-rc6-239-g0b42677e2e5d/plan/lc-compliance/
> >
> > [2] https://linux.kernelci.org/test/
> >
> > I wrote a blog post on Collabora's blog [3] going over the process of adding
> > lc-compliance to KernelCI, so check it out for more details on that. At the
> > moment lc-compliance is being compiled from my tree [4], but as soon as this
> > series get merged we can easily change KernelCI to compile from upstream.
> >
> > [3] https://www.collabora.com/news-and-blog/blog/2021/06/15/testing-cameras-with-lc-compliance-on-kernelci/
> >
> > [4] https://gitlab.collabora.com/nfraprado/libcamera/-/tree/lcc-gtest-v5
> >
> > lc-compliance Usage
> > -------------------
> >
> > lc-compliance is built around testing a single camera (and possibly optional
> > secondary cameras in the future), which is passed through the -c flag. The list
> > of tests can be queried with:
> >
> > lc-compliance --list
> >
> > By default all tests are run, but it's possible to select which tests to run by
> > using:
> >
> > lc-compliance -c '<camera_name>' --filter '<test_filter>'
> >
> > where the test_filter is in the same format as Googletest's --gtest_filter.
> > Quoting its help:
> >
> > --gtest_filter=POSTIVE_PATTERNS[-NEGATIVE_PATTERNS]
> > Run only the tests whose name matches one of the positive patterns but
> > none of the negative patterns. '?' matches any single character; '*'
> > matches any substring; ':' separates two patterns.
> >
> > This is a snippet of a successful run:
> >
> > [==========] Running 120 tests from 1 test suite.
> > [----------] Global test environment set-up.
> > [----------] 120 tests from CaptureTests/SingleStream
> > [ RUN ] CaptureTests/SingleStream.Capture/Raw_1
> > [31:47:27.239018193] [240317] INFO Camera camera.cpp:912 configuring streams: (0) 1280x720-MJPEG
> > Camera needs 4 requests, can't test only 1
> > ../src/lc-compliance/simple_capture.cpp:90: Skipped
> >
> > [ SKIPPED ] CaptureTests/SingleStream.Capture/Raw_1 (62 ms)
> >
> > [...]
> >
> > [ RUN ] CaptureTests/SingleStream.UnbalancedStop/Viewfinder_89
> > [31:50:19.266015455] [240317] INFO Camera camera.cpp:912 configuring streams: (0) 1280x720-MJPEG
> > [ OK ] CaptureTests/SingleStream.UnbalancedStop/Viewfinder_89 (3142 ms)
> > [----------] 120 tests from CaptureTests/SingleStream (175170 ms total)
> >
> > [----------] Global test environment tear-down
> > [==========] 120 tests from 1 test suite ran. (175170 ms total)
> > [ PASSED ] 96 tests.
> > [ SKIPPED ] 24 tests, listed below:
> > [ SKIPPED ] CaptureTests/SingleStream.Capture/Raw_1
> > [ SKIPPED ] CaptureTests/SingleStream.Capture/Raw_2
> > [ SKIPPED ] CaptureTests/SingleStream.Capture/Raw_3
> > [ SKIPPED ] CaptureTests/SingleStream.Capture/StillCapture_1
> >
> > [...]
> >
> > And how a failure looks like:
> >
> > [ RUN ] CaptureTests/SingleStream.Capture/Raw_5
> > [31:55:30.651452784] [241401] INFO Camera camera.cpp:912 configuring streams: (0) 1280x720-MJPEG
> > ../src/lc-compliance/simple_capture.cpp:117: Failure
> > Expected equality of these values:
> > captureCount_
> > Which is: 5
> > captureLimit_
> > Which is: 6
> > [ FAILED ] CaptureTests/SingleStream.Capture/Raw_5, where GetParam() = (0, 5) (317 ms)
> >
> > The current complete list of tests is shown below:
> >
> > CaptureTests/SingleStream.
> > Capture/Raw_1 # GetParam() = (0, 1)
> > Capture/Raw_2 # GetParam() = (0, 2)
> > Capture/Raw_3 # GetParam() = (0, 3)
> > Capture/Raw_5 # GetParam() = (0, 5)
> > Capture/Raw_8 # GetParam() = (0, 8)
> > Capture/Raw_13 # GetParam() = (0, 13)
> > Capture/Raw_21 # GetParam() = (0, 21)
> > Capture/Raw_34 # GetParam() = (0, 34)
> > Capture/Raw_55 # GetParam() = (0, 55)
> > Capture/Raw_89 # GetParam() = (0, 89)
> > Capture/StillCapture_1 # GetParam() = (1, 1)
> > Capture/StillCapture_2 # GetParam() = (1, 2)
> > Capture/StillCapture_3 # GetParam() = (1, 3)
> > Capture/StillCapture_5 # GetParam() = (1, 5)
> > Capture/StillCapture_8 # GetParam() = (1, 8)
> > Capture/StillCapture_13 # GetParam() = (1, 13)
> > Capture/StillCapture_21 # GetParam() = (1, 21)
> > Capture/StillCapture_34 # GetParam() = (1, 34)
> > Capture/StillCapture_55 # GetParam() = (1, 55)
> > Capture/StillCapture_89 # GetParam() = (1, 89)
> > Capture/VideoRecording_1 # GetParam() = (2, 1)
> > Capture/VideoRecording_2 # GetParam() = (2, 2)
> > Capture/VideoRecording_3 # GetParam() = (2, 3)
> > Capture/VideoRecording_5 # GetParam() = (2, 5)
> > Capture/VideoRecording_8 # GetParam() = (2, 8)
> > Capture/VideoRecording_13 # GetParam() = (2, 13)
> > Capture/VideoRecording_21 # GetParam() = (2, 21)
> > Capture/VideoRecording_34 # GetParam() = (2, 34)
> > Capture/VideoRecording_55 # GetParam() = (2, 55)
> > Capture/VideoRecording_89 # GetParam() = (2, 89)
> > Capture/Viewfinder_1 # GetParam() = (3, 1)
> > Capture/Viewfinder_2 # GetParam() = (3, 2)
> > Capture/Viewfinder_3 # GetParam() = (3, 3)
> > Capture/Viewfinder_5 # GetParam() = (3, 5)
> > Capture/Viewfinder_8 # GetParam() = (3, 8)
> > Capture/Viewfinder_13 # GetParam() = (3, 13)
> > Capture/Viewfinder_21 # GetParam() = (3, 21)
> > Capture/Viewfinder_34 # GetParam() = (3, 34)
> > Capture/Viewfinder_55 # GetParam() = (3, 55)
> > Capture/Viewfinder_89 # GetParam() = (3, 89)
> > CaptureStartStop/Raw_1 # GetParam() = (0, 1)
> > CaptureStartStop/Raw_2 # GetParam() = (0, 2)
> > CaptureStartStop/Raw_3 # GetParam() = (0, 3)
> > CaptureStartStop/Raw_5 # GetParam() = (0, 5)
> > CaptureStartStop/Raw_8 # GetParam() = (0, 8)
> > CaptureStartStop/Raw_13 # GetParam() = (0, 13)
> > CaptureStartStop/Raw_21 # GetParam() = (0, 21)
> > CaptureStartStop/Raw_34 # GetParam() = (0, 34)
> > CaptureStartStop/Raw_55 # GetParam() = (0, 55)
> > CaptureStartStop/Raw_89 # GetParam() = (0, 89)
> > CaptureStartStop/StillCapture_1 # GetParam() = (1, 1)
> > CaptureStartStop/StillCapture_2 # GetParam() = (1, 2)
> > CaptureStartStop/StillCapture_3 # GetParam() = (1, 3)
> > CaptureStartStop/StillCapture_5 # GetParam() = (1, 5)
> > CaptureStartStop/StillCapture_8 # GetParam() = (1, 8)
> > CaptureStartStop/StillCapture_13 # GetParam() = (1, 13)
> > CaptureStartStop/StillCapture_21 # GetParam() = (1, 21)
> > CaptureStartStop/StillCapture_34 # GetParam() = (1, 34)
> > CaptureStartStop/StillCapture_55 # GetParam() = (1, 55)
> > CaptureStartStop/StillCapture_89 # GetParam() = (1, 89)
> > CaptureStartStop/VideoRecording_1 # GetParam() = (2, 1)
> > CaptureStartStop/VideoRecording_2 # GetParam() = (2, 2)
> > CaptureStartStop/VideoRecording_3 # GetParam() = (2, 3)
> > CaptureStartStop/VideoRecording_5 # GetParam() = (2, 5)
> > CaptureStartStop/VideoRecording_8 # GetParam() = (2, 8)
> > CaptureStartStop/VideoRecording_13 # GetParam() = (2, 13)
> > CaptureStartStop/VideoRecording_21 # GetParam() = (2, 21)
> > CaptureStartStop/VideoRecording_34 # GetParam() = (2, 34)
> > CaptureStartStop/VideoRecording_55 # GetParam() = (2, 55)
> > CaptureStartStop/VideoRecording_89 # GetParam() = (2, 89)
> > CaptureStartStop/Viewfinder_1 # GetParam() = (3, 1)
> > CaptureStartStop/Viewfinder_2 # GetParam() = (3, 2)
> > CaptureStartStop/Viewfinder_3 # GetParam() = (3, 3)
> > CaptureStartStop/Viewfinder_5 # GetParam() = (3, 5)
> > CaptureStartStop/Viewfinder_8 # GetParam() = (3, 8)
> > CaptureStartStop/Viewfinder_13 # GetParam() = (3, 13)
> > CaptureStartStop/Viewfinder_21 # GetParam() = (3, 21)
> > CaptureStartStop/Viewfinder_34 # GetParam() = (3, 34)
> > CaptureStartStop/Viewfinder_55 # GetParam() = (3, 55)
> > CaptureStartStop/Viewfinder_89 # GetParam() = (3, 89)
> > UnbalancedStop/Raw_1 # GetParam() = (0, 1)
> > UnbalancedStop/Raw_2 # GetParam() = (0, 2)
> > UnbalancedStop/Raw_3 # GetParam() = (0, 3)
> > UnbalancedStop/Raw_5 # GetParam() = (0, 5)
> > UnbalancedStop/Raw_8 # GetParam() = (0, 8)
> > UnbalancedStop/Raw_13 # GetParam() = (0, 13)
> > UnbalancedStop/Raw_21 # GetParam() = (0, 21)
> > UnbalancedStop/Raw_34 # GetParam() = (0, 34)
> > UnbalancedStop/Raw_55 # GetParam() = (0, 55)
> > UnbalancedStop/Raw_89 # GetParam() = (0, 89)
> > UnbalancedStop/StillCapture_1 # GetParam() = (1, 1)
> > UnbalancedStop/StillCapture_2 # GetParam() = (1, 2)
> > UnbalancedStop/StillCapture_3 # GetParam() = (1, 3)
> > UnbalancedStop/StillCapture_5 # GetParam() = (1, 5)
> > UnbalancedStop/StillCapture_8 # GetParam() = (1, 8)
> > UnbalancedStop/StillCapture_13 # GetParam() = (1, 13)
> > UnbalancedStop/StillCapture_21 # GetParam() = (1, 21)
> > UnbalancedStop/StillCapture_34 # GetParam() = (1, 34)
> > UnbalancedStop/StillCapture_55 # GetParam() = (1, 55)
> > UnbalancedStop/StillCapture_89 # GetParam() = (1, 89)
> > UnbalancedStop/VideoRecording_1 # GetParam() = (2, 1)
> > UnbalancedStop/VideoRecording_2 # GetParam() = (2, 2)
> > UnbalancedStop/VideoRecording_3 # GetParam() = (2, 3)
> > UnbalancedStop/VideoRecording_5 # GetParam() = (2, 5)
> > UnbalancedStop/VideoRecording_8 # GetParam() = (2, 8)
> > UnbalancedStop/VideoRecording_13 # GetParam() = (2, 13)
> > UnbalancedStop/VideoRecording_21 # GetParam() = (2, 21)
> > UnbalancedStop/VideoRecording_34 # GetParam() = (2, 34)
> > UnbalancedStop/VideoRecording_55 # GetParam() = (2, 55)
> > UnbalancedStop/VideoRecording_89 # GetParam() = (2, 89)
> > UnbalancedStop/Viewfinder_1 # GetParam() = (3, 1)
> > UnbalancedStop/Viewfinder_2 # GetParam() = (3, 2)
> > UnbalancedStop/Viewfinder_3 # GetParam() = (3, 3)
> > UnbalancedStop/Viewfinder_5 # GetParam() = (3, 5)
> > UnbalancedStop/Viewfinder_8 # GetParam() = (3, 8)
> > UnbalancedStop/Viewfinder_13 # GetParam() = (3, 13)
> > UnbalancedStop/Viewfinder_21 # GetParam() = (3, 21)
> > UnbalancedStop/Viewfinder_34 # GetParam() = (3, 34)
> > UnbalancedStop/Viewfinder_55 # GetParam() = (3, 55)
> > UnbalancedStop/Viewfinder_89 # GetParam() = (3, 89)
> >
> > Changes in v11:
> > - Thanks to Paul:
> > - Updated statemachine test so that it always expects Camera::stop() to pass
> >
> > v10: https://lists.libcamera.org/pipermail/libcamera-devel/2021-June/021891.html
> >
> > Changes in v10:
> > - Thanks to Jacopo:
> > - Fixed some code style issues
> > - Stopped calling CameraManager::stop() in the path it didn't start()
> > - Updated comment header
> > - Changed ASSERT_FALSE() uses to ASSERT_EQ(...,0) where it made sense
> > - Updated copyright headers
> > - Improved "Test single capture cycles" test description
> > - Thanks to Paul:
> > - Added flag in meson.build to enable exceptions when compiling lc-compliance
> >
> > v9: https://lists.libcamera.org/pipermail/libcamera-devel/2021-June/021753.html
> >
> > Changes in v9:
> > - Thanks to Jacopo:
> > - Removed the Harness class, substituting methods with static helper functions
> > - Added EXPECT() for the number of buffers allocated in SimpleCapture::start()
> > - Changed ASSERT_FALSE() to ASSERT_EQ(..., 0)
> > - Changed tests naming
> > - Renamed single_stream.cpp to capture_test.cpp
> > - Fixed some style issues
> > - Added to --help that further Googletest options can be passed as environment
> > variables
> > - Thanks to Paul:
> > - Fixed compiling error on clang
> >
> > v8: https://lists.libcamera.org/pipermail/libcamera-devel/2021-June/021398.html
> >
> > Changes in v8:
> > - Thanks to Laurent:
> > - Fixed lc-compliance's meson.build to disable when gtest is not installed
> > - Fixed compiling errors in Clang
> > - Thanks to Kieran:
> > - Fixed coding style issues
> > - Switched from malloc/free to new/delete in gtest parameter allocation
> >
> > v7: https://lists.libcamera.org/pipermail/libcamera-devel/2021-June/021253.html
> >
> > Changes in v7:
> > - Thanks to Jacopo:
> > - Fixed style issue
> > - Made CameraManager a unique_ptr and passed to Environment as raw pointer
> > - Moved the check for buffers allocated to the beginning of
> > SimpleCapture::stop()
> > - Thanks to Niklas:
> > - Removed intermediary filter string variable in Harness::initGtestParameters()
> >
> > v6: https://lists.libcamera.org/pipermail/libcamera-devel/2021-June/021137.html
> >
> > Changes in v6:
> > - Thanks to Niklas:
> > - Made return of Environment::cameraId() be a const reference
> > - Changed buildGtestParameters() into initGtestParameters() and removed the
> > need for Harness::gtestArgc_ and Harness::gtestArgv_
> >
> > v5: https://lists.libcamera.org/pipermail/libcamera-devel/2021-May/020759.html
> >
> > Changes in v5:
> > - Thanks to Laurent:
> > - Fixed style issues
> > - Stored CameraManager and cameraId instead of Camera in Environment, and
> > acquire and release the camera for each test
> > - Thanks to Niklas:
> > - Moved buildGtestParameters() inside run()
> > - Thanks to Laurent & Niklas:
> > - Improved Environment singleton class instantiation and destruction
> >
> > v4: https://lists.libcamera.org/pipermail/libcamera-devel/2021-May/020594.html
> >
> > Changes in v4:
> > - Removed old lc-compliance results classes
> > - Thanks to Niklas:
> > - Added patch 3 with the environment singleton in its own files
> > - Improved naming of tests
> > - Thanks to Jacopo:
> > - Added \todo in Camera::stop()
> >
> > v3: https://lists.libcamera.org/pipermail/libcamera-devel/2021-May/020382.html
> >
> > Changes in v3:
> > - Thanks to Niklas:
> > - Went back to static test registration, and created a Singleton Environment
> > class to store the camera global variable
> > - Added patch 4 implementing list and filter parameters
> > - Added a nameParameters() function to give meaningful names to the test
> > parameters, removing the need to register each test suite for every role
> >
> > v2: https://lists.libcamera.org/pipermail/libcamera-devel/2021-May/020056.html
> >
> > Changes in v2:
> > - Added patch 1 to make Camera::stop() idempotent as well from [5].
> > - Made test registration dynamic in patch 3
> > - Some other minor fixes
> >
> > [5] https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019735.html
> >
> > v1: https://lists.libcamera.org/pipermail/libcamera-devel/2021-April/019702.html
> >
> > Nícolas F. R. A. Prado (5):
> > libcamera: camera: Make stop() idempotent
> > lc-compliance: Make SimpleCapture::stop() idempotent
> > lc-compliance: Add Environment singleton
> > lc-compliance: Refactor using Googletest
> > lc-compliance: Add list and filter parameters
> >
> > src/lc-compliance/capture_test.cpp | 128 ++++++++++++++++++++
> > src/lc-compliance/environment.cpp | 20 ++++
> > src/lc-compliance/environment.h | 31 +++++
> > src/lc-compliance/main.cpp | 171 +++++++++++++++++----------
> > src/lc-compliance/meson.build | 9 +-
> > src/lc-compliance/results.cpp | 75 ------------
> > src/lc-compliance/results.h | 47 --------
> > src/lc-compliance/simple_capture.cpp | 95 ++++++---------
> > src/lc-compliance/simple_capture.h | 11 +-
> > src/lc-compliance/single_stream.cpp | 97 ---------------
> > src/lc-compliance/tests.h | 16 ---
> > src/libcamera/camera.cpp | 20 +++-
> > test/camera/statemachine.cpp | 15 +--
> > 13 files changed, 358 insertions(+), 377 deletions(-)
> > create mode 100644 src/lc-compliance/capture_test.cpp
> > create mode 100644 src/lc-compliance/environment.cpp
> > create mode 100644 src/lc-compliance/environment.h
> > delete mode 100644 src/lc-compliance/results.cpp
> > delete mode 100644 src/lc-compliance/results.h
> > delete mode 100644 src/lc-compliance/single_stream.cpp
> > delete mode 100644 src/lc-compliance/tests.h
> >
> > --
> > 2.32.0
> >
More information about the libcamera-devel
mailing list