[libcamera-devel] [PATCH 32/31] test: controls: control_value: Expand test to cover all control types

[Kieran Bingham]

Hi Laurent,

On 01/03/2020 19:26, Laurent Pinchart wrote:
> The ControlValueTest hasn't been updated for a long time and is
> outdated. Improve it to support all control types, and test the type(),
> isArray() and toString() methods.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>  src/libcamera/controls.cpp      |   2 +-
>  test/controls/control_value.cpp | 110 +++++++++++++++++++++++++-------
>  2 files changed, 89 insertions(+), 23 deletions(-)
> 
> diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
> index a9dfc53e8565..aa3e7a267303 100644
> --- a/src/libcamera/controls.cpp
> +++ b/src/libcamera/controls.cpp
> @@ -202,7 +202,7 @@ std::string ControlValue::toString() const
>  		switch (type_) {
>  		case ControlTypeBool: {
>  			const bool *value = reinterpret_cast<const bool *>(data);
> -			str += *value ? "True" : "False";
> +			str += *value ? "true" : "false";


This is not a change under test: and instead modifies the toString
method of the implementation instead.

Is that intentional?

I don't see any change below checking this, and I don't see a reason to
change this to lowercase ...

>  			break;
>  		}
>  		case ControlTypeInteger8: {
> diff --git a/test/controls/control_value.cpp b/test/controls/control_value.cpp
> index a1ffa842f29e..111b4c5d1a08 100644
> --- a/test/controls/control_value.cpp
> +++ b/test/controls/control_value.cpp
> @@ -19,46 +19,112 @@ class ControlValueTest : public Test
>  protected:
>  	int run()
>  	{
> -		ControlValue integer(1234);
> -		ControlValue boolean(true);
> +		/*
> +		 * None type.
> +		 */
> +		ControlValue value;
> +		if (!value.isNone() || value.isArray()) {
> +			cerr << "Empty value is non-null" << endl;
> +			return TestFail;
> +		}
>  
> -		/* Just a string conversion output test... no validation */
> -		cout << "Int: " << integer.toString()
> -		     << " Bool: " << boolean.toString()
> -		     << endl;
> +		/*
> +		 * Bool type.
> +		 */
> +		value.set(true);
> +		if (value.isNone() || value.isArray() ||
> +		    value.type() != ControlTypeBool) {
> +			cerr << "Control type mismatch after setting to bool" << endl;
> +			return TestFail;
> +		}
>  
> -		if (integer.get<int32_t>() != 1234) {
> -			cerr << "Failed to get Integer" << endl;
> +		if (value.get<bool>() != true) {
> +			cerr << "Control value mismatch after setting to bool" << endl;
>  			return TestFail;
>  		}
>  
> -		if (boolean.get<bool>() != true) {
> -			cerr << "Failed to get Boolean" << endl;
> +		if (value.toString() != "true") {
> +			cerr << "Control string mismatch after setting to bool" << endl;
>  			return TestFail;
>  		}
>  
> -		/* Test an uninitialised value, and updating it. */
> +		/*
> +		 * Integer8 type.
> +		 */
> +		value.set(static_cast<int8_t>(42));
> +		if (value.isNone() || value.isArray() ||
> +		    value.type() != ControlTypeInteger8) {
> +			cerr << "Control type mismatch after setting to int8_t" << endl;
> +			return TestFail;
> +		}
>  
> -		ControlValue value;
> -		if (!value.isNone()) {
> -			cerr << "Empty value is non-null" << endl;
> +		if (value.get<int8_t>() != 42) {
> +			cerr << "Control value mismatch after setting to int8_t" << endl;
>  			return TestFail;
>  		}
>  
> -		value.set<bool>(true);
> -		if (value.isNone()) {
> -			cerr << "Failed to set an empty object" << endl;
> +		if (value.toString() != "42") {
> +			cerr << "Control string mismatch after setting to int8_t" << endl;
>  			return TestFail;
>  		}
>  
> -		if (value.get<bool>() != true) {
> -			cerr << "Failed to get Booleans" << endl;
> +		/*
> +		 * Integer32 type.
> +		 */
> +		value.set(0x42000000);
> +		if (value.isNone() || value.isArray() ||
> +		    value.type() != ControlTypeInteger32) {
> +			cerr << "Control type mismatch after setting to int32_t" << endl;
> +			return TestFail;
> +		}
> +
> +		if (value.get<int32_t>() != 0x42000000) {
> +			cerr << "Control value mismatch after setting to int32_t" << endl;
> +			return TestFail;
> +		}
> +
> +		if (value.toString() != "1107296256") {
> +			cerr << "Control string mismatch after setting to int32_t" << endl;
> +			return TestFail;
> +		}
> +
> +		/*
> +		 * Integer64 type.
> +		 */
> +		value.set(static_cast<int64_t>(-42));
> +		if (value.isNone() || value.isArray() ||
> +		    value.type() != ControlTypeInteger64) {
> +			cerr << "Control type mismatch after setting to int64_t" << endl;
> +			return TestFail;
> +		}
> +
> +		if (value.get<int64_t>() != -42) {
> +			cerr << "Control value mismatch after setting to int64_t" << endl;
> +			return TestFail;
> +		}
> +
> +		if (value.toString() != "-42") {
> +			cerr << "Control string mismatch after setting to int64_t" << endl;
> +			return TestFail;
> +		}
> +
> +		/*
> +		 * Float type.
> +		 */
> +		value.set(-0.42f);
> +		if (value.isNone() || value.isArray() ||
> +		    value.type() != ControlTypeFloat) {
> +			cerr << "Control type mismatch after setting to float" << endl;
> +			return TestFail;
> +		}
> +
> +		if (value.get<float>() != -0.42f) {
> +			cerr << "Control value mismatch after setting to float" << endl;
>  			return TestFail;
>  		}
>  
> -		value.set<int32_t>(10);
> -		if (value.get<int32_t>() != 10) {
> -			cerr << "Failed to get Integer" << endl;
> +		if (value.toString() != "-0.420000") {
> +			cerr << "Control string mismatch after setting to float" << endl;
>  			return TestFail;
>  		}
>  
> 

-- 
Regards
--
Kieran