[PATCH 26.11 0/4] extra unit tests for cfgfile library

[PATCH 26.11 0/4] extra unit tests for cfgfile library

[Bruce Richardson]

There were a few APIs in the cfgfile library which were not being
tested by the unit tests. Add tests to close that, and other, gaps.

Bruce Richardson (4):
  test/cfgfile: improve coverage for listing APIs
  test/cfgfile: validate config creation APIs
  test/cfgfile: verify file modification API
  test/cfgfile: test for long lines in file

 app/test/test_cfgfile.c                  | 197 +++++++++++++++++++++++
 app/test/test_cfgfiles/line_too_long.ini |   2 +-
 2 files changed, 198 insertions(+), 1 deletion(-)

--
2.53.0

[PATCH 26.11 1/4] test/cfgfile: improve coverage for listing APIs

[Bruce Richardson]

Improve cfgfile unit-test coverage for listing/index APIs, by adding
assertions for section and entry listing behavior, including:

* expected count validation
* listing sections and entries and checking expected results
* index-based listing checks
* sentinel verification to ensure only expected slots are written

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/test_cfgfile.c | 53 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/app/test/test_cfgfile.c b/app/test/test_cfgfile.c
index 1e56473064..2f1c8ec423 100644
--- a/app/test/test_cfgfile.c
+++ b/app/test/test_cfgfile.c
@@ -93,7 +93,14 @@ static int
 test_cfgfile_sample1(void)
 {
 	struct rte_cfgfile *cfgfile;
+	struct rte_cfgfile_entry entries[4];
 	char filename[PATH_MAX];
+	char sec0[CFG_NAME_LEN] = {0};
+	char sec1[CFG_NAME_LEN] = {0};
+	char sec2[CFG_NAME_LEN] = "sentinel_section_2";
+	char sec3[CFG_NAME_LEN] = "sentinel_section_3";
+	char index_sec[CFG_NAME_LEN] = {0};
+	char *sections[] = { sec0, sec1, sec2, sec3 };
 	int ret;
 
 	ret = make_tmp_file(filename, "sample1", sample1_ini);
@@ -105,6 +112,52 @@ test_cfgfile_sample1(void)
 	ret = _test_cfgfile_sample(cfgfile);
 	TEST_ASSERT_SUCCESS(ret, "Failed to validate sample file: %d", ret);
 
+	ret = rte_cfgfile_num_sections(cfgfile, NULL, 0);
+	TEST_ASSERT(ret == 2, "Unexpected number of sections: %d", ret);
+
+	ret = rte_cfgfile_sections(cfgfile, sections, 4);
+	TEST_ASSERT(ret == 2, "Unexpected listed sections: %d", ret);
+	TEST_ASSERT(strcmp(sec0, "section1") == 0,
+			"Unexpected section at index 0: %s", sec0);
+	TEST_ASSERT(strcmp(sec1, "section2") == 0,
+			"Unexpected section at index 1: %s", sec1);
+	TEST_ASSERT(strcmp(sec2, "sentinel_section_2") == 0,
+			"Unexpected write past listed sections at index 2: %s", sec2);
+	TEST_ASSERT(strcmp(sec3, "sentinel_section_3") == 0,
+			"Unexpected write past listed sections at index 3: %s", sec3);
+
+	ret = rte_cfgfile_section_num_entries_by_index(cfgfile, index_sec, 0);
+	TEST_ASSERT(ret == 1, "Unexpected entry count at index 0: %d", ret);
+	TEST_ASSERT(strcmp(index_sec, "section1") == 0,
+			"Unexpected section name at index 0: %s", index_sec);
+
+	ret = rte_cfgfile_section_num_entries(cfgfile, "section2");
+	TEST_ASSERT(ret == 2, "Unexpected section2 entry count: %d", ret);
+
+	memset(entries, 0x5a, sizeof(entries));
+	ret = rte_cfgfile_section_entries(cfgfile, "section2", entries, 4);
+	TEST_ASSERT(ret == 2, "Unexpected section2 entry count: %d", ret);
+	TEST_ASSERT(strcmp(entries[0].name, "key2") == 0,
+			"Unexpected section2 first key: %s", entries[0].name);
+	TEST_ASSERT(strcmp(entries[0].value, "value2") == 0,
+			"Unexpected section2 first value: %s", entries[0].value);
+	TEST_ASSERT(strcmp(entries[1].name, "key3") == 0,
+			"Unexpected section2 second key: %s", entries[1].name);
+	TEST_ASSERT(strcmp(entries[1].value, "value3") == 0,
+			"Unexpected section2 second value: %s", entries[1].value);
+	TEST_ASSERT((unsigned char)entries[2].name[0] == 0x5a,
+			"Unexpected write past listed entries at index 2");
+	TEST_ASSERT((unsigned char)entries[3].name[0] == 0x5a,
+			"Unexpected write past listed entries at index 3");
+
+	memset(entries, 0x5a, sizeof(entries));
+	memset(index_sec, 0, sizeof(index_sec));
+	ret = rte_cfgfile_section_entries_by_index(cfgfile, 1, index_sec, entries, 4);
+	TEST_ASSERT(ret == 2,
+			"Unexpected entry count for section at index 1: %d", ret);
+	TEST_ASSERT(strcmp(index_sec, "section2") == 0,
+			"Unexpected section name at index 1: %s", index_sec);
+
 	ret = rte_cfgfile_close(cfgfile);
 	TEST_ASSERT_SUCCESS(ret, "Failed to close cfgfile");
 
-- 
2.53.0

[PATCH 26.11 2/4] test/cfgfile: validate config creation APIs

[Bruce Richardson]

Add a test case creating a new configfile and then saving that to disk.
Verify the result by reading the file back in and checking entries are
as expected.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/test_cfgfile.c | 69 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)

diff --git a/app/test/test_cfgfile.c b/app/test/test_cfgfile.c
index 2f1c8ec423..5ed116866d 100644
--- a/app/test/test_cfgfile.c
+++ b/app/test/test_cfgfile.c
@@ -197,6 +197,74 @@ test_cfgfile_sample2(void)
 	return 0;
 }
 
+static int
+test_cfgfile_create_add_save_reload(void)
+{
+	struct rte_cfgfile *cfgfile;
+	struct rte_cfgfile *loaded;
+	const char *value;
+	char filename[PATH_MAX];
+	int ret;
+
+	cfgfile = rte_cfgfile_create(0);
+	TEST_ASSERT_NOT_NULL(cfgfile, "Failed to create cfgfile");
+
+	ret = rte_cfgfile_add_section(cfgfile, "section1");
+	TEST_ASSERT_SUCCESS(ret, "Failed to add section1");
+	ret = rte_cfgfile_add_entry(cfgfile, "section1", "key1", "value1");
+	TEST_ASSERT_SUCCESS(ret, "Failed to add section1 key1");
+	ret = rte_cfgfile_add_entry(cfgfile, "section1", "key2", "value2");
+	TEST_ASSERT_SUCCESS(ret, "Failed to add section1 key2");
+
+	ret = rte_cfgfile_add_section(cfgfile, "section2");
+	TEST_ASSERT_SUCCESS(ret, "Failed to add section2");
+	ret = rte_cfgfile_add_entry(cfgfile, "section2", "key3", "value3");
+	TEST_ASSERT_SUCCESS(ret, "Failed to add section2 key3");
+	ret = rte_cfgfile_add_entry(cfgfile, "section2", "key4", "value4");
+	TEST_ASSERT_SUCCESS(ret, "Failed to add section2 key4");
+
+	ret = make_tmp_file(filename, "create_save", "");
+	TEST_ASSERT_SUCCESS(ret, "Failed to make temporary output file");
+
+	ret = rte_cfgfile_save(cfgfile, filename);
+	TEST_ASSERT_SUCCESS(ret, "Failed to save cfgfile");
+
+	ret = rte_cfgfile_close(cfgfile);
+	TEST_ASSERT_SUCCESS(ret, "Failed to close created cfgfile");
+
+	loaded = rte_cfgfile_load(filename, 0);
+	TEST_ASSERT_NOT_NULL(loaded, "Failed to load saved cfgfile");
+
+	ret = rte_cfgfile_num_sections(loaded, NULL, 0);
+	TEST_ASSERT(ret == 2, "Unexpected number of sections: %d", ret);
+
+	ret = rte_cfgfile_section_num_entries(loaded, "section1");
+	TEST_ASSERT(ret == 2, "Unexpected section1 entries: %d", ret);
+	ret = rte_cfgfile_section_num_entries(loaded, "section2");
+	TEST_ASSERT(ret == 2, "Unexpected section2 entries: %d", ret);
+
+	value = rte_cfgfile_get_entry(loaded, "section1", "key1");
+	TEST_ASSERT(strcmp("value1", value) == 0,
+			"section1 key1 unexpected value: %s", value);
+	value = rte_cfgfile_get_entry(loaded, "section1", "key2");
+	TEST_ASSERT(strcmp("value2", value) == 0,
+			"section1 key2 unexpected value: %s", value);
+	value = rte_cfgfile_get_entry(loaded, "section2", "key3");
+	TEST_ASSERT(strcmp("value3", value) == 0,
+			"section2 key3 unexpected value: %s", value);
+	value = rte_cfgfile_get_entry(loaded, "section2", "key4");
+	TEST_ASSERT(strcmp("value4", value) == 0,
+			"section2 key4 unexpected value: %s", value);
+
+	ret = rte_cfgfile_close(loaded);
+	TEST_ASSERT_SUCCESS(ret, "Failed to close loaded cfgfile");
+
+	ret = remove(filename);
+	TEST_ASSERT_SUCCESS(ret, "Failed to remove file");
+
+	return 0;
+}
+
 static int
 test_cfgfile_realloc_sections(void)
 {
@@ -437,6 +505,7 @@ unit_test_suite test_cfgfile_suite  = {
 		TEST_CASE(test_cfgfile_missing_section),
 		TEST_CASE(test_cfgfile_global_properties),
 		TEST_CASE(test_cfgfile_empty_file),
+		TEST_CASE(test_cfgfile_create_add_save_reload),
 
 		TEST_CASES_END()
 	}
-- 
2.53.0

[PATCH 26.11 3/4] test/cfgfile: verify file modification API

[Bruce Richardson]

Check that the has_entry API correctly reports the presence of a valid
entry, and then verify that if we use set_entry we can modify the value
- a modification that persists if we use the save API. In the same test,
also check that we can't use add_entry to modify an existing entry, and
that we can't use set_entry to add a missing entry.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/test_cfgfile.c | 55 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/app/test/test_cfgfile.c b/app/test/test_cfgfile.c
index 5ed116866d..12da87aa5a 100644
--- a/app/test/test_cfgfile.c
+++ b/app/test/test_cfgfile.c
@@ -491,6 +491,60 @@ test_cfgfile_empty_file(void)
 	return 0;
 }
 
+static int
+test_cfgfile_modify_entry(void)
+{
+	struct rte_cfgfile *cfgfile;
+	struct rte_cfgfile *loaded;
+	const char *value;
+	char filename[PATH_MAX];
+	int ret;
+
+	ret = make_tmp_file(filename, "sample1_set", sample1_ini);
+	TEST_ASSERT_SUCCESS(ret, "Failed to setup temp file");
+
+	cfgfile = rte_cfgfile_load(filename, 0);
+	TEST_ASSERT_NOT_NULL(cfgfile, "Failed to load config file");
+
+	ret = rte_cfgfile_has_entry(cfgfile, "section2", "key2");
+	TEST_ASSERT(ret == 1, "section2 key2 entry missing");
+
+	ret = rte_cfgfile_has_entry(cfgfile, "section2", "invalid_key");
+	TEST_ASSERT(ret == 0, "section2 'invalid_key' entry should be missing");
+
+	ret = rte_cfgfile_set_entry(cfgfile, "section2", "key2", "value_of_key2");
+	TEST_ASSERT_SUCCESS(ret, "Failed to set section2 key2");
+
+	/* check we can't set a nonexistent key */
+	ret = rte_cfgfile_set_entry(cfgfile, "section2", "invalid_key", "value_of_key4");
+	TEST_ASSERT(ret < 0, "Error, unexpectedly able to set nonexistent 'invalid_key'");
+
+	/* check we can't add an existing key */
+	ret = rte_cfgfile_add_entry(cfgfile, "section2", "key2", "value_of_key2");
+	TEST_ASSERT(ret < 0, "Error, unexpectedly able to add existing key2");
+
+	ret = rte_cfgfile_save(cfgfile, filename);
+	TEST_ASSERT_SUCCESS(ret, "Failed to save cfgfile");
+
+	ret = rte_cfgfile_close(cfgfile);
+	TEST_ASSERT_SUCCESS(ret, "Failed to close cfgfile");
+
+	loaded = rte_cfgfile_load(filename, 0);
+	TEST_ASSERT_NOT_NULL(loaded, "Failed to reload saved cfgfile");
+
+	value = rte_cfgfile_get_entry(loaded, "section2", "key2");
+	TEST_ASSERT(strcmp("value_of_key2", value) == 0,
+			"Unexpected section2 key2 value: %s", value);
+
+	ret = rte_cfgfile_close(loaded);
+	TEST_ASSERT_SUCCESS(ret, "Failed to close reloaded cfgfile");
+
+	ret = remove(filename);
+	TEST_ASSERT_SUCCESS(ret, "Failed to remove file");
+
+	return 0;
+}
+
 static struct
 unit_test_suite test_cfgfile_suite  = {
 	.suite_name = "Test Cfgfile Unit Test Suite",
@@ -506,6 +560,7 @@ unit_test_suite test_cfgfile_suite  = {
 		TEST_CASE(test_cfgfile_global_properties),
 		TEST_CASE(test_cfgfile_empty_file),
 		TEST_CASE(test_cfgfile_create_add_save_reload),
+		TEST_CASE(test_cfgfile_modify_entry),
 
 		TEST_CASES_END()
 	}
-- 
2.53.0

[PATCH 26.11 4/4] test/cfgfile: test for long lines in file

[Bruce Richardson]

The test_cfgfiles directory already had a line_too_long.ini file in it,
but this a) did not exceed the line length that the library could manage
and b) was not actually used in any test case.

Fix this by expanding the long line so it does trigger an error, and
adding a test case for it.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/test_cfgfile.c                  | 20 ++++++++++++++++++++
 app/test/test_cfgfiles/line_too_long.ini |  2 +-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/app/test/test_cfgfile.c b/app/test/test_cfgfile.c
index 12da87aa5a..f056fe2d4d 100644
--- a/app/test/test_cfgfile.c
+++ b/app/test/test_cfgfile.c
@@ -376,6 +376,25 @@ test_cfgfile_invalid_key_value_pair(void)
 	return 0;
 }
 
+static int
+test_cfgfile_line_too_long(void)
+{
+	struct rte_cfgfile *cfgfile;
+	char filename[PATH_MAX];
+	int ret;
+
+	ret = make_tmp_file(filename, "line_too_long", line_too_long_ini);
+	TEST_ASSERT_SUCCESS(ret, "Failed to setup temp file");
+
+	cfgfile = rte_cfgfile_load(filename, 0);
+	TEST_ASSERT_NULL(cfgfile, "Expected failure did not occur");
+
+	ret = remove(filename);
+	TEST_ASSERT_SUCCESS(ret, "Failed to remove file");
+
+	return 0;
+}
+
 static int
 test_cfgfile_empty_key_value_pair(void)
 {
@@ -555,6 +574,7 @@ unit_test_suite test_cfgfile_suite  = {
 		TEST_CASE(test_cfgfile_invalid_section_header),
 		TEST_CASE(test_cfgfile_invalid_comment),
 		TEST_CASE(test_cfgfile_invalid_key_value_pair),
+		TEST_CASE(test_cfgfile_line_too_long),
 		TEST_CASE(test_cfgfile_empty_key_value_pair),
 		TEST_CASE(test_cfgfile_missing_section),
 		TEST_CASE(test_cfgfile_global_properties),
diff --git a/app/test/test_cfgfiles/line_too_long.ini b/app/test/test_cfgfiles/line_too_long.ini
index 1dce164838..2683cd2938 100644
--- a/app/test/test_cfgfiles/line_too_long.ini
+++ b/app/test/test_cfgfiles/line_too_long.ini
@@ -1,3 +1,3 @@
 [section1]
 ; this is section 1
-012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
+0123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456789
-- 
2.53.0