Message ID | 20250130115001.1129305-19-pobrn@protonmail.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
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 > >
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 > > > > >
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 */