Feedback-ID: i1ff147bf:Fastmail
From: =?utf-8?Q?Dagfinn_Ilmari_Manns=C3=A5ker?= <ilmari@ilmari.org>
To: Michael Paquier <michael@paquier.xyz>
Cc: Peter Smith <smithpb2250@gmail.com>,  Andrew Dunstan
 <andrew@dunslane.net>,  Tom Lane <tgl@sss.pgh.pa.us>,  PostgreSQL Hackers
 <pgsql-hackers@lists.postgresql.org>
Subject: Re: pg_createsubscriber TAP test wrapping makes command options
 hard to read.
References: <0f5256ec-d63e-4e0f-9b71-750a7a4e5c01@dunslane.net>
	<651128.1734017715@sss.pgh.pa.us> <87jzc46d8u.fsf@wibble.ilmari.org>
	<5f41d6c6-1cbb-494d-a5c4-3c449fc4e9ad@dunslane.net>
	<105f432a-6b0b-42d4-9061-65d263279be7@app.fastmail.com>
	<CAHut+PuD0aQfjAN0q_7FpdNdSEdrhONpTsb0tQi5ZEtZHj6bKg@mail.gmail.com>
	<87ldv542fk.fsf@wibble.ilmari.org>
	<CAHut+PvKrQXY7kM8ct=HC9xYPC6yhdJWnvCgijtgxmO-mus8uQ@mail.gmail.com>
	<Z49PYgXnbyU6IiEe@paquier.xyz> <87frlc45gg.fsf@wibble.ilmari.org>
	<Z5CHN6WajoZP6Rqz@paquier.xyz>
Date: Thu, 23 Jan 2025 20:25:45 +0000
In-Reply-To: <Z5CHN6WajoZP6Rqz@paquier.xyz> (Michael Paquier's message of
	"Wed, 22 Jan 2025 14:50:47 +0900")
Message-ID: <878qr146ra.fsf@wibble.ilmari.org>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="=-=-="
Archived-At: <https://www.postgresql.org/message-id/878qr146ra.fsf%40wibble.ilmari.org>
Precedence: bulk

--=-=-=
Content-Type: text/plain

Michael Paquier <michael@paquier.xyz> writes:

> Note that there are a few more under "Locale provider tests" in the
> initdb tests.  I've left that as a follow-up thing, that was already a
> lot..

Here's a patch for that.

- ilmari


--=-=-=
Content-Type: text/x-diff
Content-Disposition: inline;
 filename=0001-initdb-convert-more-TAP-test-to-long-option-fat-comm.patch

From d787bf86ded9f66e12325a7460761d6d10fccbe0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilmari@ilmari.org>
Date: Wed, 22 Jan 2025 16:06:17 +0000
Subject: [PATCH] initdb: convert more TAP test to long option fat comma style

---
 src/bin/initdb/t/001_initdb.pl | 116 ++++++++++++++++++++++-----------
 1 file changed, 79 insertions(+), 37 deletions(-)

diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 5cff5ce5e4d..b4d35e93c42 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -50,8 +50,7 @@
 	# while we are here, also exercise --text-search-config and --set options
 	command_ok(
 		[
-			'initdb',
-			'--no-sync',
+			'initdb', '--no-sync',
 			'--text-search-config' => 'german',
 			'--set' => 'default_text_search_config=german',
 			'--waldir' => $xlogdir,
@@ -101,8 +100,7 @@
 	# Init a new db with group access
 	my $datadir_group = "$tempdir/data_group";
 
-	command_ok(
-		[ 'initdb', '-g', $datadir_group ],
+	command_ok([ 'initdb', '--allow-group-access', $datadir_group ],
 		'successful creation with group access');
 
 	ok(check_mode_recursive($datadir_group, 0750, 0640),
@@ -114,14 +112,19 @@
 if ($ENV{with_icu} eq 'yes')
 {
 	command_fails_like(
-		[ 'initdb', '--no-sync', '--locale-provider=icu', "$tempdir/data2" ],
+		[
+			'initdb', '--no-sync',
+			'--locale-provider' => 'icu',
+			"$tempdir/data2"
+		],
 		qr/initdb: error: locale must be specified if provider is icu/,
 		'locale provider ICU requires --icu-locale');
 
 	command_ok(
 		[
 			'initdb', '--no-sync',
-			'--locale-provider=icu', '--icu-locale=en',
+			'--locale-provider' => 'icu',
+			'--icu-locale' => 'en',
 			"$tempdir/data3"
 		],
 		'option --icu-locale');
@@ -129,11 +132,15 @@
 	command_like(
 		[
 			'initdb', '--no-sync',
-			'-A' => 'trust',
-			'--locale-provider=icu', '--locale=und',
-			'--lc-collate=C', '--lc-ctype=C',
-			'--lc-messages=C', '--lc-numeric=C',
-			'--lc-monetary=C', '--lc-time=C',
+			'--auth' => 'trust',
+			'--locale-provider' => 'icu',
+			'--locale' => 'und',
+			'--lc-collate' => 'C',
+			'--lc-ctype' => 'C',
+			'--lc-messages' => 'C',
+			'--lc-numeric' => 'C',
+			'--lc-monetary' => 'C',
+			'--lc-time' => 'C',
 			"$tempdir/data4"
 		],
 		qr/^\s+default collation:\s+und\n/ms,
@@ -142,7 +149,8 @@
 	command_fails_like(
 		[
 			'initdb', '--no-sync',
-			'--locale-provider=icu', '--icu-locale=@colNumeric=lower',
+			'--locale-provider' => 'icu',
+			'--icu-locale' => '@colNumeric=lower',
 			"$tempdir/dataX"
 		],
 		qr/could not open collator for locale/,
@@ -151,8 +159,10 @@
 	command_fails_like(
 		[
 			'initdb', '--no-sync',
-			'--locale-provider=icu', '--encoding=SQL_ASCII',
-			'--icu-locale=en', "$tempdir/dataX"
+			'--locale-provider' => 'icu',
+			'--encoding' => 'SQL_ASCII',
+			'--icu-locale' => 'en',
+			"$tempdir/dataX"
 		],
 		qr/error: encoding mismatch/,
 		'fails for encoding not supported by ICU');
@@ -160,7 +170,8 @@
 	command_fails_like(
 		[
 			'initdb', '--no-sync',
-			'--locale-provider=icu', '--icu-locale=nonsense-nowhere',
+			'--locale-provider' => 'icu',
+			'--icu-locale' => 'nonsense-nowhere',
 			"$tempdir/dataX"
 		],
 		qr/error: locale "nonsense-nowhere" has unknown language "nonsense"/,
@@ -169,7 +180,8 @@
 	command_fails_like(
 		[
 			'initdb', '--no-sync',
-			'--locale-provider=icu', '--icu-locale=@colNumeric=lower',
+			'--locale-provider' => 'icu',
+			'--icu-locale' => '@colNumeric=lower',
 			"$tempdir/dataX"
 		],
 		qr/could not open collator for locale "und-u-kn-lower": U_ILLEGAL_ARGUMENT_ERROR/,
@@ -178,18 +190,27 @@
 else
 {
 	command_fails(
-		[ 'initdb', '--no-sync', '--locale-provider=icu', "$tempdir/data2" ],
+		[
+			'initdb', '--no-sync',
+			'--locale-provider' => 'icu',
+			"$tempdir/data2"
+		],
 		'locale provider ICU fails since no ICU support');
 }
 
 command_fails(
-	[ 'initdb', '--no-sync', '--locale-provider=builtin', "$tempdir/data6" ],
+	[
+		'initdb', '--no-sync',
+		'--locale-provider' => 'builtin',
+		"$tempdir/data6"
+	],
 	'locale provider builtin fails without --locale');
 
 command_ok(
 	[
 		'initdb', '--no-sync',
-		'--locale-provider=builtin', '--locale=C',
+		'--locale-provider' => 'builtin',
+		'--locale' => 'C',
 		"$tempdir/data7"
 	],
 	'locale provider builtin with --locale');
@@ -197,18 +218,24 @@
 command_ok(
 	[
 		'initdb', '--no-sync',
-		'--locale-provider=builtin', '-E UTF-8',
-		'--lc-collate=C', '--lc-ctype=C',
-		'--builtin-locale=C.UTF-8', "$tempdir/data8"
+		'--locale-provider' => 'builtin',
+		'--encoding' => 'UTF-8',
+		'--lc-collate' => 'C',
+		'--lc-ctype' => 'C',
+		'--builtin-locale' => 'C.UTF-8',
+		"$tempdir/data8"
 	],
-	'locale provider builtin with -E UTF-8 --builtin-locale=C.UTF-8');
+	'locale provider builtin with --encoding=UTF-8 --builtin-locale=C.UTF-8');
 
 command_fails(
 	[
 		'initdb', '--no-sync',
-		'--locale-provider=builtin', '-E SQL_ASCII',
-		'--lc-collate=C', '--lc-ctype=C',
-		'--builtin-locale=C.UTF-8', "$tempdir/data9"
+		'--locale-provider' => 'builtin',
+		'--encoding' => 'SQL_ASCII',
+		'--lc-collate' => 'C',
+		'--lc-ctype' => 'C',
+		'--builtin-locale' => 'C.UTF-8',
+		"$tempdir/data9"
 	],
 	'locale provider builtin with --builtin-locale=C.UTF-8 fails for SQL_ASCII'
 );
@@ -216,15 +243,18 @@
 command_ok(
 	[
 		'initdb', '--no-sync',
-		'--locale-provider=builtin', '--lc-ctype=C',
-		'--locale=C', "$tempdir/data10"
+		'--locale-provider' => 'builtin',
+		'--lc-ctype' => 'C',
+		'--locale' => 'C',
+		"$tempdir/data10"
 	],
 	'locale provider builtin with --lc-ctype');
 
 command_fails(
 	[
 		'initdb', '--no-sync',
-		'--locale-provider=builtin', '--icu-locale=en',
+		'--locale-provider' => 'builtin',
+		'--icu-locale' => 'en',
 		"$tempdir/dataX"
 	],
 	'fails for locale provider builtin with ICU locale');
@@ -232,35 +262,47 @@
 command_fails(
 	[
 		'initdb', '--no-sync',
-		'--locale-provider=builtin', '--icu-rules=""',
+		'--locale-provider' => 'builtin',
+		'--icu-rules' => '""',
 		"$tempdir/dataX"
 	],
 	'fails for locale provider builtin with ICU rules');
 
 command_fails(
-	[ 'initdb', '--no-sync', '--locale-provider=xyz', "$tempdir/dataX" ],
+	[
+		'initdb', '--no-sync',
+		'--locale-provider' => 'xyz',
+		"$tempdir/dataX"
+	],
 	'fails for invalid locale provider');
 
 command_fails(
 	[
 		'initdb', '--no-sync',
-		'--locale-provider=libc', '--icu-locale=en',
+		'--locale-provider' => 'libc',
+		'--icu-locale' => 'en',
 		"$tempdir/dataX"
 	],
 	'fails for invalid option combination');
 
 command_fails(
-	[ 'initdb', '--no-sync', '--set' => 'foo=bar', "$tempdir/dataX" ],
+	[
+		'initdb', '--no-sync',
+		'--set' => 'foo=bar',
+		"$tempdir/dataX"
+	],
 	'fails for invalid --set option');
 
-# Make sure multiple invocations of -c parameters are added case insensitive
+# Make sure multiple invocations of --set parameters are added case insensitive
 command_ok(
 	[
-		'initdb', '-cwork_mem=128',
-		'-cWork_Mem=256', '-cWORK_MEM=512',
+		'initdb', '--no-sync',
+		'--set' => 'work_mem=128',
+		'--set' => 'Work_Mem=256',
+		'--set' => 'WORK_MEM=512',
 		"$tempdir/dataY"
 	],
-	'multiple -c options with different case');
+	'multiple ---set options with different case');
 
 my $conf = slurp_file("$tempdir/dataY/postgresql.conf");
 ok($conf !~ qr/^WORK_MEM = /m, "WORK_MEM should not be configured");
-- 
2.39.5


--=-=-=--