git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v4 2/6] t/helper/test-config: return exit codes consistently

From
Matheus Tavares <matheus.bernardino@usp.br>
Date
Jun 12, 2020, 15:45 UTC
Message-ID
<85c429ac6924fefcd6b93ccefbdd52903929bb02.1591974940.git.matheus.bernardino@usp.br>
In-Reply-To
<cover.1591974940.git.matheus.bernardino@usp.br>

The test-config helper may exit with a variety of at least four different codes, to reflect the status of the requested operations. These codes are sometimes checked in the tests, but not all of the codes are returned consistently by the helper: 1 will usually refer to a "value not found", but usage errors can also return 1 or 128. Moreover, 128 is also expected on errors within the configset functions. These inconsistent uses of the exit codes can lead to false positives in the tests. Although all tests which expect errors and check the helper's exit code currently also check the output, it's still better to standardize the exit codes and avoid future problems in new tests. While we are here, let's also check that we have the expected argc for configset_get_value and configset_get_value_multi, before trying to use argv.

Note: this change is implemented with the unification of the exit
labels. This might seem unnecessary, for now, but it will benefit the
next patch, which will increase the cleanup section.
Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
---
 t/helper/test-config.c | 76 ++++++++++++++++++++++--------------------
 1 file changed, 40 insertions(+), 36 deletions(-)
diff --git a/t/helper/test-config.c b/t/helper/test-config.c
index 234c722b48..1c8e965840 100644
--- a/t/helper/test-config.c
+++ b/t/helper/test-config.c
@@ -30,6 +30,14 @@
  * iterate -> iterate over all values using git_config(), and print some
  *            data for each
  *
+ * Exit codes:
+ *     0:   success
+ *     1:   value not found for the given config key
+ *     2:   config file path given as argument is inaccessible or doesn't exist
+ *     129: test-config usage error
+ *
+ * Note: tests may also expect 128 for die() calls in the config machinery.
+ *
  * Examples:
  *
  * To print the value with highest priority for key "foo.bAr Baz.rock":
@@ -64,35 +72,42 @@ static int early_config_cb(const char *var, const char *value, void *vdata)
 	return 0;
 }
 
+enum test_config_exit_code {
+	TC_SUCCESS = 0,
+	TC_VALUE_NOT_FOUND = 1,
+	TC_CONFIG_FILE_ERROR = 2,
+	TC_USAGE_ERROR = 129,
+};
+
 int cmd__config(int argc, const char **argv)
 {
 	int i, val;
 	const char *v;
 	const struct string_list *strptr;
 	struct config_set cs;
+	enum test_config_exit_code ret = TC_SUCCESS;
 
 	if (argc == 3 && !strcmp(argv[1], "read_early_config")) {
 		read_early_config(early_config_cb, (void *)argv[2]);
-		return 0;
+		return TC_SUCCESS;
 	}
 
 	setup_git_directory();
 
 	git_configset_init(&cs);
 
-	if (argc < 2) {
-		fprintf(stderr, "Please, provide a command name on the command-line\n");
-		goto exit1;
-	} else if (argc == 3 && !strcmp(argv[1], "get_value")) {
+	if (argc < 2)
+		goto print_usage_error;
+
+	if (argc == 3 && !strcmp(argv[1], "get_value")) {
 		if (!git_config_get_value(argv[2], &v)) {
 			if (!v)
 				printf("(NULL)\n");
 			else
 				printf("%s\n", v);
-			goto exit0;
 		} else {
 			printf("Value not found for \"%s\"\n", argv[2]);
-			goto exit1;
+			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (argc == 3 && !strcmp(argv[1], "get_value_multi")) {
 		strptr = git_config_get_value_multi(argv[2]);
@@ -104,41 +119,38 @@ int cmd__config(int argc, const char **argv)
 				else
 					printf("%s\n", v);
 			}
-			goto exit0;
 		} else {
 			printf("Value not found for \"%s\"\n", argv[2]);
-			goto exit1;
+			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (argc == 3 && !strcmp(argv[1], "get_int")) {
 		if (!git_config_get_int(argv[2], &val)) {
 			printf("%d\n", val);
-			goto exit0;
 		} else {
 			printf("Value not found for \"%s\"\n", argv[2]);
-			goto exit1;
+			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (argc == 3 && !strcmp(argv[1], "get_bool")) {
 		if (!git_config_get_bool(argv[2], &val)) {
 			printf("%d\n", val);
-			goto exit0;
 		} else {
 			printf("Value not found for \"%s\"\n", argv[2]);
-			goto exit1;
+			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (argc == 3 && !strcmp(argv[1], "get_string")) {
 		if (!git_config_get_string_const(argv[2], &v)) {
 			printf("%s\n", v);
-			goto exit0;
 		} else {
 			printf("Value not found for \"%s\"\n", argv[2]);
-			goto exit1;
+			ret = TC_VALUE_NOT_FOUND;
 		}
-	} else if (!strcmp(argv[1], "configset_get_value")) {
+	} else if (argc >= 3 && !strcmp(argv[1], "configset_get_value")) {
 		for (i = 3; i < argc; i++) {
 			int err;
 			if ((err = git_configset_add_file(&cs, argv[i]))) {
 				fprintf(stderr, "Error (%d) reading configuration file %s.\n", err, argv[i]);
-				goto exit2;
+				ret = TC_CONFIG_FILE_ERROR;
+				goto out;
 			}
 		}
 		if (!git_configset_get_value(&cs, argv[2], &v)) {
@@ -146,17 +158,17 @@ int cmd__config(int argc, const char **argv)
 				printf("(NULL)\n");
 			else
 				printf("%s\n", v);
-			goto exit0;
 		} else {
 			printf("Value not found for \"%s\"\n", argv[2]);
-			goto exit1;
+			ret = TC_VALUE_NOT_FOUND;
 		}
-	} else if (!strcmp(argv[1], "configset_get_value_multi")) {
+	} else if (argc >= 3 && !strcmp(argv[1], "configset_get_value_multi")) {
 		for (i = 3; i < argc; i++) {
 			int err;
 			if ((err = git_configset_add_file(&cs, argv[i]))) {
 				fprintf(stderr, "Error (%d) reading configuration file %s.\n", err, argv[i]);
-				goto exit2;
+				ret = TC_CONFIG_FILE_ERROR;
+				goto out;
 			}
 		}
 		strptr = git_configset_get_value_multi(&cs, argv[2]);
@@ -168,27 +180,19 @@ int cmd__config(int argc, const char **argv)
 				else
 					printf("%s\n", v);
 			}
-			goto exit0;
 		} else {
 			printf("Value not found for \"%s\"\n", argv[2]);
-			goto exit1;
+			ret = TC_VALUE_NOT_FOUND;
 		}
 	} else if (!strcmp(argv[1], "iterate")) {
 		git_config(iterate_cb, NULL);
-		goto exit0;
+	} else {
+print_usage_error:
+		fprintf(stderr, "Invalid syntax. Usage: test-tool config <cmd> [args]\n");
+		ret = TC_USAGE_ERROR;
 	}
 
-	die("%s: Please check the syntax and the function name", argv[0]);
-
-exit0:
-	git_configset_clear(&cs);
-	return 0;
-
-exit1:
-	git_configset_clear(&cs);
-	return 1;
-
-exit2:
+out:
 	git_configset_clear(&cs);
-	return 2;
+	return ret;
 }

-- 
2.26.2
Previous: Matheus TavaresNext: Matheus Tavares
Message 21 of 31 in “grep: honor sparse checkout and add option to ignore it”
  1. 0/5 grep: honor sparse checkout and add option to ignore itMatheus Tavares, May 28, 2020
  2. 1/5 doc: grep: unify info on configuration variablesMatheus Tavares, May 28, 2020
  3. 3/5 config: correctly read worktree configs in submodulesMatheus Tavares, May 28, 2020
  4. Elijah NewrenMay 30, 2020
  5. Matheus Tavares BernardinoJun 1, 2020
  6. 2/5 t/helper/test-config: return exit codes consistentlyMatheus Tavares, May 28, 2020
  7. Elijah NewrenMay 30, 2020
  8. Matheus Tavares BernardinoJun 1, 2020
  9. 4/5 grep: honor sparse checkout patternsMatheus Tavares, May 28, 2020
  10. Elijah NewrenMay 30, 2020
  11. Matheus Tavares BernardinoJun 1, 2020
  12. Elijah NewrenJun 3, 2020
  13. Matheus Tavares BernardinoJun 10, 2020
  14. 5/5 config: add setting to ignore sparsity patterns in some cmdsMatheus Tavares, May 28, 2020
  15. Elijah NewrenMay 30, 2020
  16. Matheus Tavares BernardinoJun 1, 2020
  17. Elijah NewrenJun 3, 2020
  18. Matheus Tavares BernardinoJun 10, 2020
  19. Elijah NewrenJun 11, 2020
  20. 0/6 grep: honor sparse checkout and add option to ignore itMatheus Tavares, Jun 12, 2020
  21. 2/6 t/helper/test-config: return exit codes consistentlyMatheus Tavares, Jun 12, 2020
  22. 1/6 doc: grep: unify info on configuration variablesMatheus Tavares, Jun 12, 2020
  23. 3/6 t/helper/test-config: facilitate addition of new cli optionsMatheus Tavares, Jun 12, 2020
  24. 4/6 config: correctly read worktree configs in submodulesMatheus Tavares, Jun 12, 2020
  25. Elijah NewrenJun 16, 2020
  26. Matheus Tavares BernardinoJun 21, 2020
  27. Jonathan NiederSep 1, 2020
  28. Matheus Tavares BernardinoSep 1, 2020
  29. 5/6 grep: honor sparse checkout patternsMatheus Tavares, Jun 12, 2020
  30. 6/6 config: add setting to ignore sparsity patterns in some cmdsMatheus Tavares, Jun 12, 2020
  31. Elijah NewrenJun 16, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.