[RFC,v3,18/21] apps: lc-compliance: Add multi-stream tests
diff mbox series

Message ID 20250130115001.1129305-19-pobrn@protonmail.com
State New
Headers show
Series
  • apps: lc-compliance: Multi-stream tests
Related show

Commit Message

Barnabás Pőcze Jan. 30, 2025, 11:51 a.m. UTC
Rename the `SingleStream` test to `SimpleCapture`, and extend it
to support using multiple roles. And instantiate another test suite
from the `SimpleCapture` test that tests multiple streams in one
capture session.

Co-developed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/apps/lc-compliance/tests/capture_test.cpp | 85 +++++++++++--------
 1 file changed, 48 insertions(+), 37 deletions(-)

Comments

Jacopo Mondi Feb. 6, 2025, 5:24 p.m. UTC | #1
Hi Barnabás

On Thu, Jan 30, 2025 at 11:51:35AM +0000, Barnabás Pőcze wrote:
> Rename the `SingleStream` test to `SimpleCapture`, and extend it
> to support using multiple roles. And instantiate another test suite

s/And instantiate/instantiate/

> from the `SimpleCapture` test that tests multiple streams in one
> capture session.
>
> Co-developed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/apps/lc-compliance/tests/capture_test.cpp | 85 +++++++++++--------
>  1 file changed, 48 insertions(+), 37 deletions(-)
>
> diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp
> index 147e17019..db1d52fc9 100644
> --- a/src/apps/lc-compliance/tests/capture_test.cpp
> +++ b/src/apps/lc-compliance/tests/capture_test.cpp
> @@ -8,7 +8,7 @@
>
>  #include "capture.h"
>
> -#include <iostream>
> +#include <sstream>
>
>  #include <gtest/gtest.h>
>
> @@ -18,19 +18,10 @@ namespace {
>
>  using namespace libcamera;
>
> -const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
> -
> -const StreamRole ROLES[] = {
> -	StreamRole::Raw,
> -	StreamRole::StillCapture,
> -	StreamRole::VideoRecording,
> -	StreamRole::Viewfinder
> -};
> -
> -class SingleStream : public testing::TestWithParam<std::tuple<StreamRole, int>>
> +class SimpleCapture : public testing::TestWithParam<std::tuple<std::vector<StreamRole>, int>>

#include <tuple>
#include <vector>

vector should probably be included in capture.h


>  {
>  public:
> -	static std::string nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info);
> +	static std::string nameParameters(const testing::TestParamInfo<SimpleCapture::ParamType> &info);

was
#include <string>

missing ?

>
>  protected:
>  	void SetUp() override;
> @@ -43,7 +34,7 @@ protected:
>   * We use gtest's SetUp() and TearDown() instead of constructor and destructor
>   * in order to be able to assert on them.
>   */
> -void SingleStream::SetUp()
> +void SimpleCapture::SetUp()
>  {
>  	Environment *env = Environment::get();
>
> @@ -52,7 +43,7 @@ void SingleStream::SetUp()
>  	ASSERT_EQ(camera_->acquire(), 0);
>  }
>
> -void SingleStream::TearDown()
> +void SimpleCapture::TearDown()
>  {
>  	if (!camera_)
>  		return;
> @@ -61,19 +52,17 @@ void SingleStream::TearDown()
>  	camera_.reset();
>  }
>
> -std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info)
> +std::string SimpleCapture::nameParameters(const testing::TestParamInfo<SimpleCapture::ParamType> &info)
>  {
> -	std::map<StreamRole, std::string> rolesMap = {
> -		{ StreamRole::Raw, "Raw" },
> -		{ StreamRole::StillCapture, "StillCapture" },
> -		{ StreamRole::VideoRecording, "VideoRecording" },
> -		{ StreamRole::Viewfinder, "Viewfinder" }
> -	};
> +	const auto &[roles, numRequests] = info.param;
> +	std::ostringstream ss;
>
> -	std::string roleName = rolesMap[std::get<0>(info.param)];
> -	std::string numRequestsName = std::to_string(std::get<1>(info.param));
> +	for (StreamRole r : roles)
> +		ss << r << '_';
>
> -	return roleName + "_" + numRequestsName;
> +	ss << '_' << numRequests;
> +
> +	return std::move(ss).str();

Does moving ss make any difference here ?

This apart
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j

>  }
>
>  /*
> @@ -83,13 +72,13 @@ std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStre
>   * failure is a camera that completes less requests than the number of requests
>   * queued.
>   */
> -TEST_P(SingleStream, Capture)
> +TEST_P(SimpleCapture, Capture)
>  {
> -	auto [role, numRequests] = GetParam();
> +	const auto &[roles, numRequests] = GetParam();
>
>  	Capture capture(camera_);
>
> -	capture.configure(std::array{ role });
> +	capture.configure(roles);
>
>  	capture.run(numRequests, numRequests);
>  }
> @@ -101,14 +90,14 @@ TEST_P(SingleStream, Capture)
>   * a camera that does not clean up correctly in its error path but is only
>   * tested by single-capture applications.
>   */
> -TEST_P(SingleStream, CaptureStartStop)
> +TEST_P(SimpleCapture, CaptureStartStop)
>  {
> -	auto [role, numRequests] = GetParam();
> +	const auto &[roles, numRequests] = GetParam();
>  	unsigned int numRepeats = 3;
>
>  	Capture capture(camera_);
>
> -	capture.configure(std::array{ role });
> +	capture.configure(roles);
>
>  	for (unsigned int starts = 0; starts < numRepeats; starts++)
>  		capture.run(numRequests, numRequests);
> @@ -121,21 +110,43 @@ TEST_P(SingleStream, CaptureStartStop)
>   * is a camera that does not handle cancelation of buffers coming back from the
>   * video device while stopping.
>   */
> -TEST_P(SingleStream, UnbalancedStop)
> +TEST_P(SimpleCapture, UnbalancedStop)
>  {
> -	auto [role, numRequests] = GetParam();
> +	const auto &[roles, numRequests] = GetParam();
>
>  	Capture capture(camera_);
>
> -	capture.configure(std::array{ role });
> +	capture.configure(roles);
>
>  	capture.run(numRequests);
>  }
>
> -INSTANTIATE_TEST_SUITE_P(CaptureTests,
> -			 SingleStream,
> -			 testing::Combine(testing::ValuesIn(ROLES),
> +const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
> +
> +const std::vector<StreamRole> SINGLEROLES[] = {
> +	{ StreamRole::Raw, },
> +	{ StreamRole::StillCapture, },
> +	{ StreamRole::VideoRecording, },
> +	{ StreamRole::Viewfinder, },
> +};
> +
> +const std::vector<StreamRole> MULTIROLES[] = {
> +	{ StreamRole::Raw, StreamRole::StillCapture },
> +	{ StreamRole::Raw, StreamRole::VideoRecording },
> +	{ StreamRole::StillCapture, StreamRole::VideoRecording },
> +	{ StreamRole::VideoRecording, StreamRole::VideoRecording },
> +};
> +
> +INSTANTIATE_TEST_SUITE_P(SingleStream,
> +			 SimpleCapture,
> +			 testing::Combine(testing::ValuesIn(SINGLEROLES),
> +					  testing::ValuesIn(NUMREQUESTS)),
> +			 SimpleCapture::nameParameters);
> +
> +INSTANTIATE_TEST_SUITE_P(MultiStream,
> +			 SimpleCapture,
> +			 testing::Combine(testing::ValuesIn(MULTIROLES),
>  					  testing::ValuesIn(NUMREQUESTS)),
> -			 SingleStream::nameParameters);
> +			 SimpleCapture::nameParameters);
>
>  } /* namespace */
> --
> 2.48.1
>
>
Barnabás Pőcze Feb. 10, 2025, 8:04 a.m. UTC | #2
Hi


2025. február 6., csütörtök 18:24 keltezéssel, Jacopo Mondi <jacopo.mondi@ideasonboard.com> írta:

> Hi Barnabás
>
> On Thu, Jan 30, 2025 at 11:51:35AM +0000, Barnabás Pőcze wrote:
> > Rename the `SingleStream` test to `SimpleCapture`, and extend it
> > to support using multiple roles. And instantiate another test suite
>
> s/And instantiate/instantiate/

Sorry, the intention is not clear to me. I see two options:

  (1) ... using multiple roles. Instantiate ...
  (2) ... using multiple roles; instantiate ...


>
> > from the `SimpleCapture` test that tests multiple streams in one
> > capture session.
> >
> > Co-developed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
> > Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> >  src/apps/lc-compliance/tests/capture_test.cpp | 85 +++++++++++--------
> >  1 file changed, 48 insertions(+), 37 deletions(-)
> >
> > diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp
> > index 147e17019..db1d52fc9 100644
> > --- a/src/apps/lc-compliance/tests/capture_test.cpp
> > +++ b/src/apps/lc-compliance/tests/capture_test.cpp
> > @@ -8,7 +8,7 @@
> >
> >  #include "capture.h"
> >
> > -#include <iostream>
> > +#include <sstream>
> >
> >  #include <gtest/gtest.h>
> >
> > @@ -18,19 +18,10 @@ namespace {
> >
> >  using namespace libcamera;
> >
> > -const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
> > -
> > -const StreamRole ROLES[] = {
> > -	StreamRole::Raw,
> > -	StreamRole::StillCapture,
> > -	StreamRole::VideoRecording,
> > -	StreamRole::Viewfinder
> > -};
> > -
> > -class SingleStream : public testing::TestWithParam<std::tuple<StreamRole, int>>
> > +class SimpleCapture : public testing::TestWithParam<std::tuple<std::vector<StreamRole>, int>>
>
> #include <tuple>
> #include <vector>
>
> vector should probably be included in capture.h

I'll add it.


>
>
> >  {
> >  public:
> > -	static std::string nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info);
> > +	static std::string nameParameters(const testing::TestParamInfo<SimpleCapture::ParamType> &info);
>
> was
> #include <string>
>
> missing ?

I'll add all three.


>
> >
> >  protected:
> >  	void SetUp() override;
> > @@ -43,7 +34,7 @@ protected:
> >   * We use gtest's SetUp() and TearDown() instead of constructor and destructor
> >   * in order to be able to assert on them.
> >   */
> > -void SingleStream::SetUp()
> > +void SimpleCapture::SetUp()
> >  {
> >  	Environment *env = Environment::get();
> >
> > @@ -52,7 +43,7 @@ void SingleStream::SetUp()
> >  	ASSERT_EQ(camera_->acquire(), 0);
> >  }
> >
> > -void SingleStream::TearDown()
> > +void SimpleCapture::TearDown()
> >  {
> >  	if (!camera_)
> >  		return;
> > @@ -61,19 +52,17 @@ void SingleStream::TearDown()
> >  	camera_.reset();
> >  }
> >
> > -std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info)
> > +std::string SimpleCapture::nameParameters(const testing::TestParamInfo<SimpleCapture::ParamType> &info)
> >  {
> > -	std::map<StreamRole, std::string> rolesMap = {
> > -		{ StreamRole::Raw, "Raw" },
> > -		{ StreamRole::StillCapture, "StillCapture" },
> > -		{ StreamRole::VideoRecording, "VideoRecording" },
> > -		{ StreamRole::Viewfinder, "Viewfinder" }
> > -	};
> > +	const auto &[roles, numRequests] = info.param;
> > +	std::ostringstream ss;
> >
> > -	std::string roleName = rolesMap[std::get<0>(info.param)];
> > -	std::string numRequestsName = std::to_string(std::get<1>(info.param));
> > +	for (StreamRole r : roles)
> > +		ss << r << '_';
> >
> > -	return roleName + "_" + numRequestsName;
> > +	ss << '_' << numRequests;
> > +
> > +	return std::move(ss).str();
>
> Does moving ss make any difference here ?

No, it does not change anything in C++17. It is there by accident.


>
> This apart
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
>
> Thanks
>   j
>
> >  }
> >
> >  /*
> > @@ -83,13 +72,13 @@ std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStre
> >   * failure is a camera that completes less requests than the number of requests
> >   * queued.
> >   */
> > -TEST_P(SingleStream, Capture)
> > +TEST_P(SimpleCapture, Capture)
> >  {
> > -	auto [role, numRequests] = GetParam();
> > +	const auto &[roles, numRequests] = GetParam();
> >
> >  	Capture capture(camera_);
> >
> > -	capture.configure(std::array{ role });
> > +	capture.configure(roles);
> >
> >  	capture.run(numRequests, numRequests);
> >  }
> > @@ -101,14 +90,14 @@ TEST_P(SingleStream, Capture)
> >   * a camera that does not clean up correctly in its error path but is only
> >   * tested by single-capture applications.
> >   */
> > -TEST_P(SingleStream, CaptureStartStop)
> > +TEST_P(SimpleCapture, CaptureStartStop)
> >  {
> > -	auto [role, numRequests] = GetParam();
> > +	const auto &[roles, numRequests] = GetParam();
> >  	unsigned int numRepeats = 3;
> >
> >  	Capture capture(camera_);
> >
> > -	capture.configure(std::array{ role });
> > +	capture.configure(roles);
> >
> >  	for (unsigned int starts = 0; starts < numRepeats; starts++)
> >  		capture.run(numRequests, numRequests);
> > @@ -121,21 +110,43 @@ TEST_P(SingleStream, CaptureStartStop)
> >   * is a camera that does not handle cancelation of buffers coming back from the
> >   * video device while stopping.
> >   */
> > -TEST_P(SingleStream, UnbalancedStop)
> > +TEST_P(SimpleCapture, UnbalancedStop)
> >  {
> > -	auto [role, numRequests] = GetParam();
> > +	const auto &[roles, numRequests] = GetParam();
> >
> >  	Capture capture(camera_);
> >
> > -	capture.configure(std::array{ role });
> > +	capture.configure(roles);
> >
> >  	capture.run(numRequests);
> >  }
> >
> > -INSTANTIATE_TEST_SUITE_P(CaptureTests,
> > -			 SingleStream,
> > -			 testing::Combine(testing::ValuesIn(ROLES),
> > +const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
> > +
> > +const std::vector<StreamRole> SINGLEROLES[] = {
> > +	{ StreamRole::Raw, },
> > +	{ StreamRole::StillCapture, },
> > +	{ StreamRole::VideoRecording, },
> > +	{ StreamRole::Viewfinder, },
> > +};
> > +
> > +const std::vector<StreamRole> MULTIROLES[] = {
> > +	{ StreamRole::Raw, StreamRole::StillCapture },
> > +	{ StreamRole::Raw, StreamRole::VideoRecording },
> > +	{ StreamRole::StillCapture, StreamRole::VideoRecording },
> > +	{ StreamRole::VideoRecording, StreamRole::VideoRecording },
> > +};
> > +
> > +INSTANTIATE_TEST_SUITE_P(SingleStream,
> > +			 SimpleCapture,
> > +			 testing::Combine(testing::ValuesIn(SINGLEROLES),
> > +					  testing::ValuesIn(NUMREQUESTS)),
> > +			 SimpleCapture::nameParameters);
> > +
> > +INSTANTIATE_TEST_SUITE_P(MultiStream,
> > +			 SimpleCapture,
> > +			 testing::Combine(testing::ValuesIn(MULTIROLES),
> >  					  testing::ValuesIn(NUMREQUESTS)),
> > -			 SingleStream::nameParameters);
> > +			 SimpleCapture::nameParameters);
> >
> >  } /* namespace */
> > --
> > 2.48.1
> >
> >
>

Patch
diff mbox series

diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp
index 147e17019..db1d52fc9 100644
--- a/src/apps/lc-compliance/tests/capture_test.cpp
+++ b/src/apps/lc-compliance/tests/capture_test.cpp
@@ -8,7 +8,7 @@ 
 
 #include "capture.h"
 
-#include <iostream>
+#include <sstream>
 
 #include <gtest/gtest.h>
 
@@ -18,19 +18,10 @@  namespace {
 
 using namespace libcamera;
 
-const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
-
-const StreamRole ROLES[] = {
-	StreamRole::Raw,
-	StreamRole::StillCapture,
-	StreamRole::VideoRecording,
-	StreamRole::Viewfinder
-};
-
-class SingleStream : public testing::TestWithParam<std::tuple<StreamRole, int>>
+class SimpleCapture : public testing::TestWithParam<std::tuple<std::vector<StreamRole>, int>>
 {
 public:
-	static std::string nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info);
+	static std::string nameParameters(const testing::TestParamInfo<SimpleCapture::ParamType> &info);
 
 protected:
 	void SetUp() override;
@@ -43,7 +34,7 @@  protected:
  * We use gtest's SetUp() and TearDown() instead of constructor and destructor
  * in order to be able to assert on them.
  */
-void SingleStream::SetUp()
+void SimpleCapture::SetUp()
 {
 	Environment *env = Environment::get();
 
@@ -52,7 +43,7 @@  void SingleStream::SetUp()
 	ASSERT_EQ(camera_->acquire(), 0);
 }
 
-void SingleStream::TearDown()
+void SimpleCapture::TearDown()
 {
 	if (!camera_)
 		return;
@@ -61,19 +52,17 @@  void SingleStream::TearDown()
 	camera_.reset();
 }
 
-std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStream::ParamType> &info)
+std::string SimpleCapture::nameParameters(const testing::TestParamInfo<SimpleCapture::ParamType> &info)
 {
-	std::map<StreamRole, std::string> rolesMap = {
-		{ StreamRole::Raw, "Raw" },
-		{ StreamRole::StillCapture, "StillCapture" },
-		{ StreamRole::VideoRecording, "VideoRecording" },
-		{ StreamRole::Viewfinder, "Viewfinder" }
-	};
+	const auto &[roles, numRequests] = info.param;
+	std::ostringstream ss;
 
-	std::string roleName = rolesMap[std::get<0>(info.param)];
-	std::string numRequestsName = std::to_string(std::get<1>(info.param));
+	for (StreamRole r : roles)
+		ss << r << '_';
 
-	return roleName + "_" + numRequestsName;
+	ss << '_' << numRequests;
+
+	return std::move(ss).str();
 }
 
 /*
@@ -83,13 +72,13 @@  std::string SingleStream::nameParameters(const testing::TestParamInfo<SingleStre
  * failure is a camera that completes less requests than the number of requests
  * queued.
  */
-TEST_P(SingleStream, Capture)
+TEST_P(SimpleCapture, Capture)
 {
-	auto [role, numRequests] = GetParam();
+	const auto &[roles, numRequests] = GetParam();
 
 	Capture capture(camera_);
 
-	capture.configure(std::array{ role });
+	capture.configure(roles);
 
 	capture.run(numRequests, numRequests);
 }
@@ -101,14 +90,14 @@  TEST_P(SingleStream, Capture)
  * a camera that does not clean up correctly in its error path but is only
  * tested by single-capture applications.
  */
-TEST_P(SingleStream, CaptureStartStop)
+TEST_P(SimpleCapture, CaptureStartStop)
 {
-	auto [role, numRequests] = GetParam();
+	const auto &[roles, numRequests] = GetParam();
 	unsigned int numRepeats = 3;
 
 	Capture capture(camera_);
 
-	capture.configure(std::array{ role });
+	capture.configure(roles);
 
 	for (unsigned int starts = 0; starts < numRepeats; starts++)
 		capture.run(numRequests, numRequests);
@@ -121,21 +110,43 @@  TEST_P(SingleStream, CaptureStartStop)
  * is a camera that does not handle cancelation of buffers coming back from the
  * video device while stopping.
  */
-TEST_P(SingleStream, UnbalancedStop)
+TEST_P(SimpleCapture, UnbalancedStop)
 {
-	auto [role, numRequests] = GetParam();
+	const auto &[roles, numRequests] = GetParam();
 
 	Capture capture(camera_);
 
-	capture.configure(std::array{ role });
+	capture.configure(roles);
 
 	capture.run(numRequests);
 }
 
-INSTANTIATE_TEST_SUITE_P(CaptureTests,
-			 SingleStream,
-			 testing::Combine(testing::ValuesIn(ROLES),
+const int NUMREQUESTS[] = { 1, 2, 3, 5, 8, 13, 21, 34, 55, 89 };
+
+const std::vector<StreamRole> SINGLEROLES[] = {
+	{ StreamRole::Raw, },
+	{ StreamRole::StillCapture, },
+	{ StreamRole::VideoRecording, },
+	{ StreamRole::Viewfinder, },
+};
+
+const std::vector<StreamRole> MULTIROLES[] = {
+	{ StreamRole::Raw, StreamRole::StillCapture },
+	{ StreamRole::Raw, StreamRole::VideoRecording },
+	{ StreamRole::StillCapture, StreamRole::VideoRecording },
+	{ StreamRole::VideoRecording, StreamRole::VideoRecording },
+};
+
+INSTANTIATE_TEST_SUITE_P(SingleStream,
+			 SimpleCapture,
+			 testing::Combine(testing::ValuesIn(SINGLEROLES),
+					  testing::ValuesIn(NUMREQUESTS)),
+			 SimpleCapture::nameParameters);
+
+INSTANTIATE_TEST_SUITE_P(MultiStream,
+			 SimpleCapture,
+			 testing::Combine(testing::ValuesIn(MULTIROLES),
 					  testing::ValuesIn(NUMREQUESTS)),
-			 SingleStream::nameParameters);
+			 SimpleCapture::nameParameters);
 
 } /* namespace */