Horton

Thread

Commits

  1. test_escape: Fix output of --help

  2. test_escape: Fix handling of short options in getopt_long()

  1. test_escape: invalid option -- 'c'

    Japin Li <japinli@hotmail.com> — 2025-02-18T03:12:13Z

    Hi, all

When I test test_escape, I find the usage of test_escape declares it
support -c option, however, when I use it, it complains it is an
invalid option.

Here is a patch to fix it.

diff --git a/src/test/modules/test_escape/test_escape.c b/src/test/modules/test_escape/test_escape.c
index 09303a00a20..7132e3a9316 100644
--- a/src/test/modules/test_escape/test_escape.c
+++ b/src/test/modules/test_escape/test_escape.c
@@ -824,7 +824,7 @@ main(int argc, char *argv[])
 		{NULL, 0, NULL, 0},
 	};
 
-	while ((c = getopt_long(argc, argv, "vqh", long_options, &option_index)) != -1)
+	while ((c = getopt_long(argc, argv, "c:vqh", long_options, &option_index)) != -1)
 	{
 		switch (c)
 		{
-- 
Regrads,
Japin Li

  2. Re: test_escape: invalid option -- 'c'

    Michael Paquier <michael@paquier.xyz> — 2025-02-18T03:46:50Z

    On Tue, Feb 18, 2025 at 11:12:13AM +0800, Japin Li wrote:
> When I test test_escape, I find the usage of test_escape declares it
> support -c option, however, when I use it, it complains it is an
> invalid option.
> 
> Here is a patch to fix it.

You are right, obviously.  The only reason why this is not showing up
is that the TAP test uses the long option --conninfo.  Speaking of
which, this had better use fat commas to combine the option/value
pairs in these commands.
--
Michael

  3. Re: test_escape: invalid option -- 'c'

    Dagfinn Ilmari Mannsåker <ilmari@ilmari.org> — 2025-02-18T13:11:11Z

    Michael Paquier <michael@paquier.xyz> writes:

> On Tue, Feb 18, 2025 at 11:12:13AM +0800, Japin Li wrote:
>> When I test test_escape, I find the usage of test_escape declares it
>> support -c option, however, when I use it, it complains it is an
>> invalid option.
>> 
>> Here is a patch to fix it.
>
> You are right, obviously.  The only reason why this is not showing up
> is that the TAP test uses the long option --conninfo.

This was something I did consider when porting the tests to use long
options: should we leave (or add) at least one usage of every
corresponding shor option, for exactly this reason?

> Speaking of which, this had better use fat commas to combine the
> option/value pairs in these commands.

I have a local branch which converts more of the tests to the long and
fat style, I'll try to finish that up for submission soon, and look at
adding/retaining coverage for short options.

- ilmari

  4. Re: test_escape: invalid option -- 'c'

    Michael Paquier <michael@paquier.xyz> — 2025-02-19T00:53:09Z

    On Tue, Feb 18, 2025 at 12:46:50PM +0900, Michael Paquier wrote:
> You are right, obviously.  The only reason why this is not showing up
> is that the TAP test uses the long option --conninfo.  Speaking of
> which, this had better use fat commas to combine the option/value
> pairs in these commands.

This was also missing 'f', which is listed as an option.  I've
reordered all these to be alphabetical in the list, and applied the
result down to v13.  Thanks for the report.
--
Michael

  5. Re: test_escape: invalid option -- 'c'

    Japin Li <japinli@hotmail.com> — 2025-02-19T02:31:40Z

    On Wed, 19 Feb 2025 at 09:53, Michael Paquier <michael@paquier.xyz> wrote:
> On Tue, Feb 18, 2025 at 12:46:50PM +0900, Michael Paquier wrote:
>> You are right, obviously.  The only reason why this is not showing up
>> is that the TAP test uses the long option --conninfo.  Speaking of
>> which, this had better use fat commas to combine the option/value
>> pairs in these commands.
>
> This was also missing 'f', which is listed as an option.  I've
> reordered all these to be alphabetical in the list, and applied the
> result down to v13.  Thanks for the report.

Thanks.

I didn't add the 'f' option since the usage() does not declare it.
If you add this, we also should update the usage(), right?

diff --git a/src/test/modules/test_escape/test_escape.c b/src/test/modules/test_escape/test_escape.c
index a8e9c3cb518..f6b36448977 100644
--- a/src/test/modules/test_escape/test_escape.c
+++ b/src/test/modules/test_escape/test_escape.c
@@ -801,7 +801,7 @@ usage(const char *hint)
 		   "  -c, --conninfo=CONNINFO   connection information to use\n"
 		   "  -v, --verbose             show test details even for successes\n"
 		   "  -q, --quiet               only show failures\n"
-		   "      --force-unsupported   test invalid input even if unsupported\n"
+		   "  -f, --force-unsupported   test invalid input even if unsupported\n"
 		);
 
 	if (hint)

-- 
Regrads,
Japin Li

  6. Re: test_escape: invalid option -- 'c'

    Michael Paquier <michael@paquier.xyz> — 2025-02-19T03:13:47Z

    On Wed, Feb 19, 2025 at 10:31:40AM +0800, Japin Li wrote:
> I didn't add the 'f' option since the usage() does not declare it.
> If you add this, we also should update the usage(), right?

Yes, we should.
--
Michael