[v3,1/3] libcamera: yaml-parser: Add additional tests
diff mbox series

Message ID 20240920132823.88433-2-stefan.klug@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: yaml-parser: Differentiate between empty and empty string
Related show

Commit Message

Stefan Klug Sept. 20, 2024, 1:28 p.m. UTC 
Add additional tests in preparation for upcoming modifications on the
yaml parser. These tests handle the case where the yaml file contains
empty items in dictionaries or lists. E.g.:

dict:
  key_with_value: value
  key_without_value:

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>

---

In the end it turned out that the changes on the YamlParser were never
wrong, so these tests are somewhat superfluous. We could still merge
them to test specifically for that case or drop the patch.

Changes in v3:
- Added seperate patch for the working tests
---
 test/yaml-parser.cpp | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

Comments

Kieran Bingham Sept. 21, 2024, 12:44 p.m. UTC | #1 
Quoting Stefan Klug (2024-09-20 14:28:08)
> Add additional tests in preparation for upcoming modifications on the
> yaml parser. These tests handle the case where the yaml file contains
> empty items in dictionaries or lists. E.g.:
> 
> dict:
>   key_with_value: value
>   key_without_value:
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 


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

> ---
> 
> In the end it turned out that the changes on the YamlParser were never
> wrong, so these tests are somewhat superfluous. We could still merge
> them to test specifically for that case or drop the patch.
> 
> Changes in v3:
> - Added seperate patch for the working tests
> ---
>  test/yaml-parser.cpp | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
> index 81c829834667..347999831d61 100644
> --- a/test/yaml-parser.cpp
> +++ b/test/yaml-parser.cpp
> @@ -34,10 +34,12 @@ static const string testYaml =
>         "list:\n"
>         "  - James\n"
>         "  - Mary\n"
> +       "  - \n"
>         "dictionary:\n"
>         "  a: 1\n"
>         "  c: 3\n"
>         "  b: 2\n"
> +       "  empty:\n"
>         "level1:\n"
>         "  level2:\n"
>         "    - [1, 2]\n"
> @@ -430,9 +432,10 @@ protected:
>                 if (testObjectType(listObj, "list", Type::List) != TestPass)
>                         return TestFail;
>  
> -               static constexpr std::array<const char *, 2> listValues{
> +               static constexpr std::array<const char *, 3> listValues{
>                         "James",
>                         "Mary",
> +                       "",
>                 };
>  
>                 if (listObj.size() != listValues.size()) {
> @@ -465,16 +468,23 @@ protected:
>                         i++;
>                 }
>  
> +               /* Ensure that empty objects get parsed as empty strings. */
> +               if (!listObj[2].isValue()) {
> +                       cerr << "Empty object is not a value" << std::endl;
> +                       return TestFail;
> +               }
> +
>                 /* Test dictionary object */
>                 auto &dictObj = (*root)["dictionary"];
>  
>                 if (testObjectType(dictObj, "dictionary", Type::Dictionary) != TestPass)
>                         return TestFail;
>  
> -               static constexpr std::array<std::pair<const char *, int>, 3> dictValues{ {
> +               static constexpr std::array<std::pair<const char *, int>, 4> dictValues{ {
>                         { "a", 1 },
>                         { "c", 3 },
>                         { "b", 2 },
> +                       { "empty", -100 },
>                 } };
>  
>                 size_t dictSize = dictValues.size();
> @@ -505,7 +515,7 @@ protected:
>                                 return TestFail;
>                         }
>  
> -                       if (elem.get<int32_t>(0) != item.second) {
> +                       if (elem.get<int32_t>(-100) != item.second) {
>                                 std::cerr << "Dictionary element " << i << " has wrong value"
>                                           << std::endl;
>                                 return TestFail;
> @@ -514,6 +524,18 @@ protected:
>                         i++;
>                 }
>  
> +               /* Ensure that empty objects get parsed as empty strings. */
> +               if (!dictObj["empty"].isValue()) {
> +                       cerr << "Empty object is not of type value" << std::endl;
> +                       return TestFail;
> +               }
> +
> +               /* Ensure that keys without values are added to a dict. */
> +               if (!dictObj.contains("empty")) {
> +                       cerr << "Empty element is missing in dict" << std::endl;
> +                       return TestFail;
> +               }
> +
>                 /* Make sure utils::map_keys() works on the adapter. */
>                 (void)utils::map_keys(dictObj.asDict());
>  
> -- 
> 2.43.0
>
Paul Elder Sept. 23, 2024, 9:50 a.m. UTC | #2 
On Fri, Sep 20, 2024 at 03:28:08PM +0200, Stefan Klug wrote:
> Add additional tests in preparation for upcoming modifications on the
> yaml parser. These tests handle the case where the yaml file contains
> empty items in dictionaries or lists. E.g.:
> 
> dict:
>   key_with_value: value
>   key_without_value:
> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> 
> ---
> 
> In the end it turned out that the changes on the YamlParser were never
> wrong, so these tests are somewhat superfluous. We could still merge
> them to test specifically for that case or drop the patch.

I think it's worth keeping the test to confirm correctness and ensure
that these aren't broken.

Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>

> 
> Changes in v3:
> - Added seperate patch for the working tests
> ---
>  test/yaml-parser.cpp | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
> 
> diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
> index 81c829834667..347999831d61 100644
> --- a/test/yaml-parser.cpp
> +++ b/test/yaml-parser.cpp
> @@ -34,10 +34,12 @@ static const string testYaml =
>  	"list:\n"
>  	"  - James\n"
>  	"  - Mary\n"
> +	"  - \n"
>  	"dictionary:\n"
>  	"  a: 1\n"
>  	"  c: 3\n"
>  	"  b: 2\n"
> +	"  empty:\n"
>  	"level1:\n"
>  	"  level2:\n"
>  	"    - [1, 2]\n"
> @@ -430,9 +432,10 @@ protected:
>  		if (testObjectType(listObj, "list", Type::List) != TestPass)
>  			return TestFail;
>  
> -		static constexpr std::array<const char *, 2> listValues{
> +		static constexpr std::array<const char *, 3> listValues{
>  			"James",
>  			"Mary",
> +			"",
>  		};
>  
>  		if (listObj.size() != listValues.size()) {
> @@ -465,16 +468,23 @@ protected:
>  			i++;
>  		}
>  
> +		/* Ensure that empty objects get parsed as empty strings. */
> +		if (!listObj[2].isValue()) {
> +			cerr << "Empty object is not a value" << std::endl;
> +			return TestFail;
> +		}
> +
>  		/* Test dictionary object */
>  		auto &dictObj = (*root)["dictionary"];
>  
>  		if (testObjectType(dictObj, "dictionary", Type::Dictionary) != TestPass)
>  			return TestFail;
>  
> -		static constexpr std::array<std::pair<const char *, int>, 3> dictValues{ {
> +		static constexpr std::array<std::pair<const char *, int>, 4> dictValues{ {
>  			{ "a", 1 },
>  			{ "c", 3 },
>  			{ "b", 2 },
> +			{ "empty", -100 },
>  		} };
>  
>  		size_t dictSize = dictValues.size();
> @@ -505,7 +515,7 @@ protected:
>  				return TestFail;
>  			}
>  
> -			if (elem.get<int32_t>(0) != item.second) {
> +			if (elem.get<int32_t>(-100) != item.second) {
>  				std::cerr << "Dictionary element " << i << " has wrong value"
>  					  << std::endl;
>  				return TestFail;
> @@ -514,6 +524,18 @@ protected:
>  			i++;
>  		}
>  
> +		/* Ensure that empty objects get parsed as empty strings. */
> +		if (!dictObj["empty"].isValue()) {
> +			cerr << "Empty object is not of type value" << std::endl;
> +			return TestFail;
> +		}
> +
> +		/* Ensure that keys without values are added to a dict. */
> +		if (!dictObj.contains("empty")) {
> +			cerr << "Empty element is missing in dict" << std::endl;
> +			return TestFail;
> +		}
> +
>  		/* Make sure utils::map_keys() works on the adapter. */
>  		(void)utils::map_keys(dictObj.asDict());
>  
> -- 
> 2.43.0
>

Patch
diff mbox series 

diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
index 81c829834667..347999831d61 100644
--- a/test/yaml-parser.cpp
+++ b/test/yaml-parser.cpp
@@ -34,10 +34,12 @@  static const string testYaml =
 	"list:\n"
 	"  - James\n"
 	"  - Mary\n"
+	"  - \n"
 	"dictionary:\n"
 	"  a: 1\n"
 	"  c: 3\n"
 	"  b: 2\n"
+	"  empty:\n"
 	"level1:\n"
 	"  level2:\n"
 	"    - [1, 2]\n"
@@ -430,9 +432,10 @@  protected:
 		if (testObjectType(listObj, "list", Type::List) != TestPass)
 			return TestFail;
 
-		static constexpr std::array<const char *, 2> listValues{
+		static constexpr std::array<const char *, 3> listValues{
 			"James",
 			"Mary",
+			"",
 		};
 
 		if (listObj.size() != listValues.size()) {
@@ -465,16 +468,23 @@  protected:
 			i++;
 		}
 
+		/* Ensure that empty objects get parsed as empty strings. */
+		if (!listObj[2].isValue()) {
+			cerr << "Empty object is not a value" << std::endl;
+			return TestFail;
+		}
+
 		/* Test dictionary object */
 		auto &dictObj = (*root)["dictionary"];
 
 		if (testObjectType(dictObj, "dictionary", Type::Dictionary) != TestPass)
 			return TestFail;
 
-		static constexpr std::array<std::pair<const char *, int>, 3> dictValues{ {
+		static constexpr std::array<std::pair<const char *, int>, 4> dictValues{ {
 			{ "a", 1 },
 			{ "c", 3 },
 			{ "b", 2 },
+			{ "empty", -100 },
 		} };
 
 		size_t dictSize = dictValues.size();
@@ -505,7 +515,7 @@  protected:
 				return TestFail;
 			}
 
-			if (elem.get<int32_t>(0) != item.second) {
+			if (elem.get<int32_t>(-100) != item.second) {
 				std::cerr << "Dictionary element " << i << " has wrong value"
 					  << std::endl;
 				return TestFail;
@@ -514,6 +524,18 @@  protected:
 			i++;
 		}
 
+		/* Ensure that empty objects get parsed as empty strings. */
+		if (!dictObj["empty"].isValue()) {
+			cerr << "Empty object is not of type value" << std::endl;
+			return TestFail;
+		}
+
+		/* Ensure that keys without values are added to a dict. */
+		if (!dictObj.contains("empty")) {
+			cerr << "Empty element is missing in dict" << std::endl;
+			return TestFail;
+		}
+
 		/* Make sure utils::map_keys() works on the adapter. */
 		(void)utils::map_keys(dictObj.asDict());