diff mbox series

[05/20] userdiff tests: list builtin drivers via test-tool

Message ID 20210215005236.11313-6-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series [01/20] userdiff: refactor away the parse_bool() function | expand

Commit Message

Ævar Arnfjörð Bjarmason Feb. 15, 2021, 12:52 a.m. UTC
Change the userdiff test to list the builtin drivers via the
test-tool, using the new for_each_userdiff_driver() API function.

This gets rid of the need to modify this part of the test every time a
new pattern is added, see 2ff6c34612 (userdiff: support Bash,
2020-10-22) and 09dad9256a (userdiff: support Markdown, 2020-05-02)
for two recent examples.

I only need the "list-builtin-drivers "argument here, but let's add
"list-custom-drivers" and "list-drivers" too, just because it's easy.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Makefile                 |  1 +
 t/helper/test-tool.c     |  1 +
 t/helper/test-tool.h     |  1 +
 t/helper/test-userdiff.c | 32 ++++++++++++++++++++++++++++++++
 t/t4018-diff-funcname.sh | 29 +++++------------------------
 5 files changed, 40 insertions(+), 24 deletions(-)
 create mode 100644 t/helper/test-userdiff.c

Comments

Eric Sunshine Feb. 15, 2021, 1:24 a.m. UTC | #1
On Sun, Feb 14, 2021 at 7:56 PM Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> Change the userdiff test to list the builtin drivers via the
> test-tool, using the new for_each_userdiff_driver() API function.
> [...]
> I only need the "list-builtin-drivers "argument here, but let's add
> "list-custom-drivers" and "list-drivers" too, just because it's easy.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> diff --git a/t/helper/test-userdiff.c b/t/helper/test-userdiff.c
> @@ -0,0 +1,32 @@
> +static int userdiff_list_builtin_drivers_cb(struct userdiff_driver *driver,
> +                                           enum userdiff_driver_type type,
> +                                           void *priv)
> +{
> +       puts(driver->name);
> +       return 0;
> +}

Nit: The word "builtin" in the name of this callback function made me
search the patch for its cousin function which would be used for
custom drivers. It was only after reading on that I discovered that
this one function is used for both builtin and custom drivers. (Caught
me off guard, but not itself worth a re-roll.)

> diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
> @@ -8,6 +8,10 @@ test_description='Test custom diff function name patterns'
>  test_expect_success 'setup' '
> +       test-tool userdiff list-builtin-drivers >builtin-drivers &&
> +       test_file_not_empty builtin-drivers &&
> +       builtin_drivers=$(cat builtin-drivers) &&

Nit: I get why you are using test_file_not_empty() for its diagnostic
value, but it does confuse the reader a bit to see the file
"builtin-drivers" created for no particularly good reason. This could
easily have been:

    builtin_drivers=$(test-tool userdiff list-builtin-drivers) &&
    test -n "$builtin_drivers" &&

Subjective and not worth a re-roll.
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index 5a239cac20..710a0deaed 100644
--- a/Makefile
+++ b/Makefile
@@ -741,6 +741,7 @@  TEST_BUILTINS_OBJS += test-submodule-nested-repo-config.o
 TEST_BUILTINS_OBJS += test-subprocess.o
 TEST_BUILTINS_OBJS += test-trace2.o
 TEST_BUILTINS_OBJS += test-urlmatch-normalization.o
+TEST_BUILTINS_OBJS += test-userdiff.o
 TEST_BUILTINS_OBJS += test-wildmatch.o
 TEST_BUILTINS_OBJS += test-windows-named-pipe.o
 TEST_BUILTINS_OBJS += test-write-cache.o
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index f97cd9f48a..dcb05ca6e5 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -71,6 +71,7 @@  static struct test_cmd cmds[] = {
 	{ "submodule-nested-repo-config", cmd__submodule_nested_repo_config },
 	{ "subprocess", cmd__subprocess },
 	{ "trace2", cmd__trace2 },
+	{ "userdiff", cmd__userdiff },
 	{ "urlmatch-normalization", cmd__urlmatch_normalization },
 	{ "xml-encode", cmd__xml_encode },
 	{ "wildmatch", cmd__wildmatch },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index 28072c0ad5..589f2e8ac6 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -61,6 +61,7 @@  int cmd__submodule_config(int argc, const char **argv);
 int cmd__submodule_nested_repo_config(int argc, const char **argv);
 int cmd__subprocess(int argc, const char **argv);
 int cmd__trace2(int argc, const char **argv);
+int cmd__userdiff(int argc, const char **argv);
 int cmd__urlmatch_normalization(int argc, const char **argv);
 int cmd__xml_encode(int argc, const char **argv);
 int cmd__wildmatch(int argc, const char **argv);
diff --git a/t/helper/test-userdiff.c b/t/helper/test-userdiff.c
new file mode 100644
index 0000000000..80de456287
--- /dev/null
+++ b/t/helper/test-userdiff.c
@@ -0,0 +1,32 @@ 
+#include "test-tool.h"
+#include "cache.h"
+#include "userdiff.h"
+
+static int userdiff_list_builtin_drivers_cb(struct userdiff_driver *driver,
+					    enum userdiff_driver_type type,
+					    void *priv)
+{
+	puts(driver->name);
+	return 0;
+}
+
+static int list_what(enum userdiff_driver_type type)
+{
+	return for_each_userdiff_driver(userdiff_list_builtin_drivers_cb, type,
+					NULL);
+}
+
+int cmd__userdiff(int argc, const char **argv)
+{
+	if (argc != 2)
+		return 1;
+
+	if (!strcmp(argv[1], "list-drivers"))
+		return list_what(USERDIFF_DRIVER_TYPE_UNSPECIFIED);
+	else if (!strcmp(argv[1], "list-builtin-drivers"))
+		return list_what(USERDIFF_DRIVER_TYPE_BUILTIN);
+	else if (!strcmp(argv[1], "list-custom-drivers"))
+		return list_what(USERDIFF_DRIVER_TYPE_CUSTOM);
+	else
+		return error("unknown argument %s", argv[1]);
+}
diff --git a/t/t4018-diff-funcname.sh b/t/t4018-diff-funcname.sh
index cefe329aea..11ac648451 100755
--- a/t/t4018-diff-funcname.sh
+++ b/t/t4018-diff-funcname.sh
@@ -8,6 +8,10 @@  test_description='Test custom diff function name patterns'
 . ./test-lib.sh
 
 test_expect_success 'setup' '
+	test-tool userdiff list-builtin-drivers >builtin-drivers &&
+	test_file_not_empty builtin-drivers &&
+	builtin_drivers=$(cat builtin-drivers) &&
+
 	# a non-trivial custom pattern
 	git config diff.custom1.funcname "!static
 !String
@@ -26,30 +30,7 @@  test_expect_success 'setup' '
 '
 
 diffpatterns="
-	ada
-	bash
-	bibtex
-	cpp
-	csharp
-	css
-	dts
-	elixir
-	fortran
-	fountain
-	golang
-	html
-	java
-	markdown
-	matlab
-	objc
-	pascal
-	perl
-	php
-	python
-	ruby
-	rust
-	tex
-	default
+	$builtin_drivers
 	custom1
 	custom2
 	custom3