[v2,1/2] libcamera: test: Add a failing test for the log level parser
diff mbox series

Message ID 20250612135943.522819-2-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • Fix log level parsing when multiple categories are listed
Related show

Commit Message

Stefan Klug June 12, 2025, 1:56 p.m. UTC 
Log level parsing doesn't always work as expected.  Add a failing test
for that.

Co-authored-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 test/log/log_api.cpp | 39 ++++++++++++++++++++++++++++++++++++++-
 test/log/meson.build |  5 +++--
 2 files changed, 41 insertions(+), 3 deletions(-)

Comments

Laurent Pinchart June 12, 2025, 2:20 p.m. UTC | #1 
On Thu, Jun 12, 2025 at 03:56:44PM +0200, Stefan Klug wrote:
> Log level parsing doesn't always work as expected.  Add a failing test
> for that.
> 
> Co-authored-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  test/log/log_api.cpp | 39 ++++++++++++++++++++++++++++++++++++++-
>  test/log/meson.build |  5 +++--
>  2 files changed, 41 insertions(+), 3 deletions(-)
> 
> diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp
> index 0b999738d891..8d19cf0ceb67 100644
> --- a/test/log/log_api.cpp
> +++ b/test/log/log_api.cpp
> @@ -26,6 +26,11 @@ using namespace std;
>  using namespace libcamera;
>  
>  LOG_DEFINE_CATEGORY(LogAPITest)
> +LOG_DEFINE_CATEGORY(Cat0)
> +LOG_DEFINE_CATEGORY(Cat1)
> +LOG_DEFINE_CATEGORY(Cat2)
> +LOG_DEFINE_CATEGORY(Cat3)
> +LOG_DEFINE_CATEGORY(Cat4)
>  
>  class LogAPITest : public Test
>  {
> @@ -74,6 +79,34 @@ protected:
>  		return TestPass;
>  	}
>  
> +	int testEnvLevels()
> +	{
> +		setenv("LIBCAMERA_LOG_LEVELS",
> +		       "Cat0:0,Cat0:9999,Cat1:INFO,Cat1:INVALID,Cat2:2,Cat2:-1,"
> +		       "Cat3:ERROR,Cat3:{[]},Cat4:4,Cat4:rubbish",
> +		       true);
> +		logSetTarget(libcamera::LoggingTargetNone);
> +
> +		const std::pair<const LogCategory &, libcamera::LogSeverity> expected[] = {
> +			{ _LOG_CATEGORY(Cat0)(), libcamera::LogDebug },
> +			{ _LOG_CATEGORY(Cat1)(), libcamera::LogInfo },
> +			{ _LOG_CATEGORY(Cat2)(), libcamera::LogWarning },
> +			{ _LOG_CATEGORY(Cat3)(), libcamera::LogError },
> +			{ _LOG_CATEGORY(Cat4)(), libcamera::LogFatal },
> +		};
> +		bool ok = true;

		int result = TestPass;

> +
> +		for (const auto &[c, s] : expected) {
> +			if (c.severity() != s) {
> +				ok = false;

				result = TestFail;

> +				cerr << "Severity of " << c.name() << " (" << c.severity() << ") "
> +				     << "does not equal " << s << endl;
> +			}
> +		}
> +
> +		return ok ? TestPass : TestFail;

		return result;

> +	}
> +
>  	int testFile()
>  	{
>  		int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);
> @@ -135,7 +168,11 @@ protected:
>  
>  	int run() override
>  	{
> -		int ret = testFile();
> +		int ret = testEnvLevels();
> +		if (ret != TestPass)
> +			return TestFail;
> +
> +		ret = testFile();
>  		if (ret != TestPass)
>  			return TestFail;
>  
> diff --git a/test/log/meson.build b/test/log/meson.build
> index 2298ff84ee62..d91f62b9ea5b 100644
> --- a/test/log/meson.build
> +++ b/test/log/meson.build
> @@ -1,7 +1,7 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  log_test = [
> -    {'name': 'log_api', 'sources': ['log_api.cpp']},
> +    {'name': 'log_api', 'sources': ['log_api.cpp'], 'should_fail':true},

Missing space before true.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>      {'name': 'log_process', 'sources': ['log_process.cpp']},
>  ]
>  
> @@ -11,5 +11,6 @@ foreach test : log_test
>                       link_with : test_libraries,
>                       include_directories : test_includes_internal)
>  
> -    test(test['name'], exe, suite : 'log')
> +    test(test['name'], exe, suite : 'log',
> +         should_fail : test.get('should_fail', false))
>  endforeach
Kieran Bingham June 13, 2025, 8:22 a.m. UTC | #2 
Quoting Laurent Pinchart (2025-06-12 15:20:48)
> On Thu, Jun 12, 2025 at 03:56:44PM +0200, Stefan Klug wrote:
> > Log level parsing doesn't always work as expected.  Add a failing test
> > for that.
> > 
> > Co-authored-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >  test/log/log_api.cpp | 39 ++++++++++++++++++++++++++++++++++++++-
> >  test/log/meson.build |  5 +++--
> >  2 files changed, 41 insertions(+), 3 deletions(-)
> > 
> > diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp
> > index 0b999738d891..8d19cf0ceb67 100644
> > --- a/test/log/log_api.cpp
> > +++ b/test/log/log_api.cpp
> > @@ -26,6 +26,11 @@ using namespace std;
> >  using namespace libcamera;
> >  
> >  LOG_DEFINE_CATEGORY(LogAPITest)
> > +LOG_DEFINE_CATEGORY(Cat0)
> > +LOG_DEFINE_CATEGORY(Cat1)
> > +LOG_DEFINE_CATEGORY(Cat2)
> > +LOG_DEFINE_CATEGORY(Cat3)
> > +LOG_DEFINE_CATEGORY(Cat4)
> >  
> >  class LogAPITest : public Test
> >  {
> > @@ -74,6 +79,34 @@ protected:
> >               return TestPass;
> >       }
> >  
> > +     int testEnvLevels()
> > +     {
> > +             setenv("LIBCAMERA_LOG_LEVELS",
> > +                    "Cat0:0,Cat0:9999,Cat1:INFO,Cat1:INVALID,Cat2:2,Cat2:-1,"
> > +                    "Cat3:ERROR,Cat3:{[]},Cat4:4,Cat4:rubbish",
> > +                    true);
> > +             logSetTarget(libcamera::LoggingTargetNone);
> > +
> > +             const std::pair<const LogCategory &, libcamera::LogSeverity> expected[] = {
> > +                     { _LOG_CATEGORY(Cat0)(), libcamera::LogDebug },
> > +                     { _LOG_CATEGORY(Cat1)(), libcamera::LogInfo },
> > +                     { _LOG_CATEGORY(Cat2)(), libcamera::LogWarning },
> > +                     { _LOG_CATEGORY(Cat3)(), libcamera::LogError },
> > +                     { _LOG_CATEGORY(Cat4)(), libcamera::LogFatal },
> > +             };
> > +             bool ok = true;
> 
>                 int result = TestPass;
> 
> > +
> > +             for (const auto &[c, s] : expected) {
> > +                     if (c.severity() != s) {
> > +                             ok = false;
> 
>                                 result = TestFail;
> 
> > +                             cerr << "Severity of " << c.name() << " (" << c.severity() << ") "
> > +                                  << "does not equal " << s << endl;
> > +                     }
> > +             }
> > +
> > +             return ok ? TestPass : TestFail;
> 
>                 return result;
> 
> > +     }
> > +
> >       int testFile()
> >       {
> >               int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);
> > @@ -135,7 +168,11 @@ protected:
> >  
> >       int run() override
> >       {
> > -             int ret = testFile();
> > +             int ret = testEnvLevels();
> > +             if (ret != TestPass)
> > +                     return TestFail;
> > +
> > +             ret = testFile();
> >               if (ret != TestPass)
> >                       return TestFail;
> >  
> > diff --git a/test/log/meson.build b/test/log/meson.build
> > index 2298ff84ee62..d91f62b9ea5b 100644
> > --- a/test/log/meson.build
> > +++ b/test/log/meson.build
> > @@ -1,7 +1,7 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >  
> >  log_test = [
> > -    {'name': 'log_api', 'sources': ['log_api.cpp']},
> > +    {'name': 'log_api', 'sources': ['log_api.cpp'], 'should_fail':true},
> 
> Missing space before true.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

I have nothign on top of that so:


Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> >      {'name': 'log_process', 'sources': ['log_process.cpp']},
> >  ]
> >  
> > @@ -11,5 +11,6 @@ foreach test : log_test
> >                       link_with : test_libraries,
> >                       include_directories : test_includes_internal)
> >  
> > -    test(test['name'], exe, suite : 'log')
> > +    test(test['name'], exe, suite : 'log',
> > +         should_fail : test.get('should_fail', false))
> >  endforeach
> 
> -- 
> Regards,
> 
> Laurent Pinchart

Patch
diff mbox series 

diff --git a/test/log/log_api.cpp b/test/log/log_api.cpp
index 0b999738d891..8d19cf0ceb67 100644
--- a/test/log/log_api.cpp
+++ b/test/log/log_api.cpp
@@ -26,6 +26,11 @@  using namespace std;
 using namespace libcamera;
 
 LOG_DEFINE_CATEGORY(LogAPITest)
+LOG_DEFINE_CATEGORY(Cat0)
+LOG_DEFINE_CATEGORY(Cat1)
+LOG_DEFINE_CATEGORY(Cat2)
+LOG_DEFINE_CATEGORY(Cat3)
+LOG_DEFINE_CATEGORY(Cat4)
 
 class LogAPITest : public Test
 {
@@ -74,6 +79,34 @@  protected:
 		return TestPass;
 	}
 
+	int testEnvLevels()
+	{
+		setenv("LIBCAMERA_LOG_LEVELS",
+		       "Cat0:0,Cat0:9999,Cat1:INFO,Cat1:INVALID,Cat2:2,Cat2:-1,"
+		       "Cat3:ERROR,Cat3:{[]},Cat4:4,Cat4:rubbish",
+		       true);
+		logSetTarget(libcamera::LoggingTargetNone);
+
+		const std::pair<const LogCategory &, libcamera::LogSeverity> expected[] = {
+			{ _LOG_CATEGORY(Cat0)(), libcamera::LogDebug },
+			{ _LOG_CATEGORY(Cat1)(), libcamera::LogInfo },
+			{ _LOG_CATEGORY(Cat2)(), libcamera::LogWarning },
+			{ _LOG_CATEGORY(Cat3)(), libcamera::LogError },
+			{ _LOG_CATEGORY(Cat4)(), libcamera::LogFatal },
+		};
+		bool ok = true;
+
+		for (const auto &[c, s] : expected) {
+			if (c.severity() != s) {
+				ok = false;
+				cerr << "Severity of " << c.name() << " (" << c.severity() << ") "
+				     << "does not equal " << s << endl;
+			}
+		}
+
+		return ok ? TestPass : TestFail;
+	}
+
 	int testFile()
 	{
 		int fd = open("/tmp", O_TMPFILE | O_RDWR, S_IRUSR | S_IWUSR);
@@ -135,7 +168,11 @@  protected:
 
 	int run() override
 	{
-		int ret = testFile();
+		int ret = testEnvLevels();
+		if (ret != TestPass)
+			return TestFail;
+
+		ret = testFile();
 		if (ret != TestPass)
 			return TestFail;
 
diff --git a/test/log/meson.build b/test/log/meson.build
index 2298ff84ee62..d91f62b9ea5b 100644
--- a/test/log/meson.build
+++ b/test/log/meson.build
@@ -1,7 +1,7 @@ 
 # SPDX-License-Identifier: CC0-1.0
 
 log_test = [
-    {'name': 'log_api', 'sources': ['log_api.cpp']},
+    {'name': 'log_api', 'sources': ['log_api.cpp'], 'should_fail':true},
     {'name': 'log_process', 'sources': ['log_process.cpp']},
 ]
 
@@ -11,5 +11,6 @@  foreach test : log_test
                      link_with : test_libraries,
                      include_directories : test_includes_internal)
 
-    test(test['name'], exe, suite : 'log')
+    test(test['name'], exe, suite : 'log',
+         should_fail : test.get('should_fail', false))
 endforeach