[libcamera-devel,v3,02/16] test: control_list: Check for Brightness presence
diff mbox series

Message ID 20210421160319.42251-3-jacopo@jmondi.org
State Accepted
Delegated to: Jacopo Mondi
Headers show
Series
  • Support SensorTimestamp metadata
Related show

Commit Message

Jacopo Mondi April 21, 2021, 4:03 p.m. UTC
The current test verifies that after adding a new control to a list
already populated with one control the new one is present.

However the test wrongly tests for its presence twice instead of
making sure the existing control is still there.

Fix this by checking for the presence of both controls after the
update, and fix the error message accordingly.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 test/controls/control_list.cpp | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Niklas Söderlund April 21, 2021, 6:11 p.m. UTC | #1
Hi Jacopo,

Thanks for your work.

On 2021-04-21 18:03:05 +0200, Jacopo Mondi wrote:
> The current test verifies that after adding a new control to a list
> already populated with one control the new one is present.
> 
> However the test wrongly tests for its presence twice instead of
> making sure the existing control is still there.
> 
> Fix this by checking for the presence of both controls after the
> update, and fix the error message accordingly.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  test/controls/control_list.cpp | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
> index b5a49dc17000..2b321ddd6fa4 100644
> --- a/test/controls/control_list.cpp
> +++ b/test/controls/control_list.cpp
> @@ -108,9 +108,10 @@ protected:
>  		list.set(controls::Brightness, 0.0f);
>  		list.set(controls::Contrast, 1.5f);
>  
> -		if (!list.contains(controls::Contrast) ||
> +		if (!list.contains(controls::Brightness) ||
>  		    !list.contains(controls::Contrast)) {
> -			cout << "List should contain Contrast control" << endl;
> +			cout << "List should contain Brightness and Contrast controls"
> +			     << endl;
>  			return TestFail;
>  		}
>  
> -- 
> 2.31.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Hirokazu Honda April 22, 2021, 3:24 a.m. UTC | #2
Hi Jacopo, thank you for the patch.

On Thu, Apr 22, 2021 at 3:11 AM Niklas Söderlund
<niklas.soderlund@ragnatech.se> wrote:
>
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2021-04-21 18:03:05 +0200, Jacopo Mondi wrote:
> > The current test verifies that after adding a new control to a list
> > already populated with one control the new one is present.
> >
> > However the test wrongly tests for its presence twice instead of
> > making sure the existing control is still there.
> >
> > Fix this by checking for the presence of both controls after the
> > update, and fix the error message accordingly.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
>

Reviewed-by: Hirokazu Honda <hiroh@chromium.org>

> > ---
> >  test/controls/control_list.cpp | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
> > index b5a49dc17000..2b321ddd6fa4 100644
> > --- a/test/controls/control_list.cpp
> > +++ b/test/controls/control_list.cpp
> > @@ -108,9 +108,10 @@ protected:
> >               list.set(controls::Brightness, 0.0f);
> >               list.set(controls::Contrast, 1.5f);
> >
> > -             if (!list.contains(controls::Contrast) ||
> > +             if (!list.contains(controls::Brightness) ||
> >                   !list.contains(controls::Contrast)) {
> > -                     cout << "List should contain Contrast control" << endl;
> > +                     cout << "List should contain Brightness and Contrast controls"
> > +                          << endl;
> >                       return TestFail;
> >               }
> >
> > --
> > 2.31.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel@lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Laurent Pinchart April 26, 2021, 4:22 a.m. UTC | #3
Hi Jacopo,

Thank you for the patch.

On Wed, Apr 21, 2021 at 06:03:05PM +0200, Jacopo Mondi wrote:
> The current test verifies that after adding a new control to a list
> already populated with one control the new one is present.
> 
> However the test wrongly tests for its presence twice instead of
> making sure the existing control is still there.
> 
> Fix this by checking for the presence of both controls after the
> update, and fix the error message accordingly.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

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

> ---
>  test/controls/control_list.cpp | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
> index b5a49dc17000..2b321ddd6fa4 100644
> --- a/test/controls/control_list.cpp
> +++ b/test/controls/control_list.cpp
> @@ -108,9 +108,10 @@ protected:
>  		list.set(controls::Brightness, 0.0f);
>  		list.set(controls::Contrast, 1.5f);
>  
> -		if (!list.contains(controls::Contrast) ||
> +		if (!list.contains(controls::Brightness) ||
>  		    !list.contains(controls::Contrast)) {
> -			cout << "List should contain Contrast control" << endl;
> +			cout << "List should contain Brightness and Contrast controls"
> +			     << endl;
>  			return TestFail;
>  		}
>

Patch
diff mbox series

diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp
index b5a49dc17000..2b321ddd6fa4 100644
--- a/test/controls/control_list.cpp
+++ b/test/controls/control_list.cpp
@@ -108,9 +108,10 @@  protected:
 		list.set(controls::Brightness, 0.0f);
 		list.set(controls::Contrast, 1.5f);
 
-		if (!list.contains(controls::Contrast) ||
+		if (!list.contains(controls::Brightness) ||
 		    !list.contains(controls::Contrast)) {
-			cout << "List should contain Contrast control" << endl;
+			cout << "List should contain Brightness and Contrast controls"
+			     << endl;
 			return TestFail;
 		}