[v4,2/2] apps: lc-compliance: Add multi-stream tests
diff mbox series

Message ID 20250314174220.1015597-3-barnabas.pocze@ideasonboard.com
State Accepted
Headers show
Series
  • apps: lc-compliance: Multi-stream tests
Related show

Commit Message

Barnabás Pőcze March 14, 2025, 5:42 p.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.

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

Comments

Jacopo Mondi March 18, 2025, 8:49 a.m. UTC | #1 
Hi Barnabás

On Fri, Mar 14, 2025 at 06:42:19PM +0100, Barnabás Pőcze wrote:
> 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.
>
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

My tag stands.

On top of this we can probably
1) Rename SimpleCapture as "Simple" doesn't tell much
2) Rework the number of requests that we iterate on as this seems
rather random

But for now, feel free to push these changes!

> ---
>  src/apps/lc-compliance/tests/capture_test.cpp | 88 +++++++++++--------
>  1 file changed, 51 insertions(+), 37 deletions(-)
>
> diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp
> index 06f15bdb4..d02caa8a1 100644
> --- a/src/apps/lc-compliance/tests/capture_test.cpp
> +++ b/src/apps/lc-compliance/tests/capture_test.cpp
> @@ -8,7 +8,10 @@
>
>  #include "capture.h"
>
> -#include <iostream>
> +#include <sstream>
> +#include <string>
> +#include <tuple>
> +#include <vector>
>
>  #include <gtest/gtest.h>
>
> @@ -18,19 +21,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 +37,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 +46,7 @@ void SingleStream::SetUp()
>  	ASSERT_EQ(camera_->acquire(), 0);
>  }
>
> -void SingleStream::TearDown()
> +void SimpleCapture::TearDown()
>  {
>  	if (!camera_)
>  		return;
> @@ -61,19 +55,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 ss.str();
>  }
>
>  /*
> @@ -83,13 +75,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({ { role } });
> +	capture.configure(roles);
>
>  	capture.run(numRequests, numRequests);
>  }
> @@ -101,14 +93,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({ { role } });
> +	capture.configure(roles);
>
>  	for (unsigned int starts = 0; starts < numRepeats; starts++)
>  		capture.run(numRequests, numRequests);
> @@ -121,21 +113,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({ { 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 March 21, 2025, 8:12 a.m. UTC | #2 
Hi


2025. 03. 18. 9:49 keltezéssel, Jacopo Mondi írta:
> Hi Barnabás
> 
> On Fri, Mar 14, 2025 at 06:42:19PM +0100, Barnabás Pőcze wrote:
>> 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.
>>
>> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
>> Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
>> Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
> 
> My tag stands.
> 
> On top of this we can probably
> 1) Rename SimpleCapture as "Simple" doesn't tell much

Yes, but it can't be called "SingleCapture" anymore, and the
"Capture" name is already taken by the test.


> 2) Rework the number of requests that we iterate on as this seems
> rather random

I preserved the earlier values; by the way, it's the Fibonacci sequence.
But maybe they should indeed be more targeted.


Regards,
Barnabás Pőcze

> 
> But for now, feel free to push these changes!
> 
>> ---
>>   src/apps/lc-compliance/tests/capture_test.cpp | 88 +++++++++++--------
>>   1 file changed, 51 insertions(+), 37 deletions(-)
>>
>> diff --git a/src/apps/lc-compliance/tests/capture_test.cpp b/src/apps/lc-compliance/tests/capture_test.cpp
>> index 06f15bdb4..d02caa8a1 100644
>> --- a/src/apps/lc-compliance/tests/capture_test.cpp
>> +++ b/src/apps/lc-compliance/tests/capture_test.cpp
>> @@ -8,7 +8,10 @@
>>
>>   #include "capture.h"
>>
>> -#include <iostream>
>> +#include <sstream>
>> +#include <string>
>> +#include <tuple>
>> +#include <vector>
>>
>>   #include <gtest/gtest.h>
>>
>> @@ -18,19 +21,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 +37,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 +46,7 @@ void SingleStream::SetUp()
>>   	ASSERT_EQ(camera_->acquire(), 0);
>>   }
>>
>> -void SingleStream::TearDown()
>> +void SimpleCapture::TearDown()
>>   {
>>   	if (!camera_)
>>   		return;
>> @@ -61,19 +55,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 ss.str();
>>   }
>>
>>   /*
>> @@ -83,13 +75,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({ { role } });
>> +	capture.configure(roles);
>>
>>   	capture.run(numRequests, numRequests);
>>   }
>> @@ -101,14 +93,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({ { role } });
>> +	capture.configure(roles);
>>
>>   	for (unsigned int starts = 0; starts < numRepeats; starts++)
>>   		capture.run(numRequests, numRequests);
>> @@ -121,21 +113,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({ { 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 06f15bdb4..d02caa8a1 100644
--- a/src/apps/lc-compliance/tests/capture_test.cpp
+++ b/src/apps/lc-compliance/tests/capture_test.cpp
@@ -8,7 +8,10 @@ 
 
 #include "capture.h"
 
-#include <iostream>
+#include <sstream>
+#include <string>
+#include <tuple>
+#include <vector>
 
 #include <gtest/gtest.h>
 
@@ -18,19 +21,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 +37,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 +46,7 @@  void SingleStream::SetUp()
 	ASSERT_EQ(camera_->acquire(), 0);
 }
 
-void SingleStream::TearDown()
+void SimpleCapture::TearDown()
 {
 	if (!camera_)
 		return;
@@ -61,19 +55,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 ss.str();
 }
 
 /*
@@ -83,13 +75,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({ { role } });
+	capture.configure(roles);
 
 	capture.run(numRequests, numRequests);
 }
@@ -101,14 +93,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({ { role } });
+	capture.configure(roles);
 
 	for (unsigned int starts = 0; starts < numRepeats; starts++)
 		capture.run(numRequests, numRequests);
@@ -121,21 +113,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({ { 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 */