After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 550433 - g_test_init doesn't recognize --help
g_test_init doesn't recognize --help
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2008-09-02 11:35 UTC by Christian Dywan
Modified: 2012-08-17 01:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add --help output (1.54 KB, patch)
2008-09-02 12:26 UTC, Christian Dywan
committed Details | Review
Improve help output (6.12 KB, patch)
2008-10-17 13:35 UTC, Christian Dywan
none Details | Review
Adkjust GTester help format, recognize -? (3.00 KB, patch)
2008-10-17 17:36 UTC, Christian Dywan
none Details | Review

Description Christian Dywan 2008-09-02 11:35:21 UTC
Currently g_test_init recognizes a bunch of arguments that are parsed manually.

Ergo -h and --help respectively don't work.

The code doesn't make use of GOptionContext.
Comment 1 Christian Dywan 2008-09-02 12:26:26 UTC
Created attachment 117842 [details] [review]
Add --help output

This patch adds recognition of -? and --help to the command line argument parsing in g_test_init and prints an output much like what g_option_context_get_help would have produced.

The output is not localized, should it be?

One line is specially tweaked so that it looks good on an 80 columns console, is that appropriate?
Comment 2 Matthias Clasen 2008-09-06 22:07:05 UTC
I'd much rather accept a patch to make use of the option parser that lives next file in the same library. Really, this should not have been accepted in this form...
Comment 3 Christian Dywan 2008-09-06 23:07:54 UTC
(In reply to comment #2)
> I'd much rather accept a patch to make use of the option parser that lives next
> file in the same library. Really, this should not have been accepted in this
> form...

You mean using GOptionContext. In fact I wanted to do that originally as you can see in the bug description, but, which I should have mentioned, I was pointed out while writing the patch, that to be able to actually test GOptionContext it cannot be used in g_test_init. ie. the code should rely on as few functionality as possible.
Comment 4 Matthias Clasen 2008-09-06 23:27:19 UTC
Ah well. Thats an argument I have to agree with, grudgingly. 
Maybe add a little comment explaining this.
Comment 5 Matthias Clasen 2008-09-07 03:22:52 UTC
Other than that, the patch looks reasonable.
Comment 6 Christian Dywan 2008-09-08 08:30:16 UTC
Committed, including a two-liner saying that GOptionContext is not used for a reason.
Comment 7 Tim Janik 2008-10-16 10:38:04 UTC
Comment on attachment 117842 [details] [review]
Add --help output

Only saw this now and have a few review bits still:

>diff --git a/glib/gtestutils.c b/glib/gtestutils.c
>index 8c0eb1f..0b77908 100644
>--- a/glib/gtestutils.c
>+++ b/glib/gtestutils.c
>@@ -347,6 +347,27 @@ parse_args (gint    *argc_p,
>             }
>           argv[i] = NULL;
>         }
>+      else if (strcmp ("-?", argv[i]) == 0 || strcmp ("--help", argv[i]) == 0)

This should definitely support '-h' as --help alias. We do that everywhere else and '-h' is a widely used abbreviatiuon for --help.

>+        {
>+          printf ("Usage:\n"
>+                  "  %s [OPTION...]\n\n"
>+                  "Help Options:\n"
>+                  "  -?, --help                     Show help options\n"
>+                  "Test Options:\n"
>+                  "  -l                             List test cases available in a test executable\n"
>+                  "  -seed=RANDOMSEED               Provide a random seed to reproduce test\n"

The docs from gtester here read:
  --seed=SEEDSTRING           start all tests with random number seed SEEDSTRING
Would be nice to keep them consistent.

>+                  "                                 runs using random numbers\n"
>+                  "  --verbose                      Run tests verbosely\n"
>+                  "  -q, --quiet                    Run tests quietly\n"
>+                  "  -p TESTPATH                    execute all tests matching TESTPATH\n"
>+                  "  -m {perf|slow|thorough|quick}  Execute tests according modes\n"
>+                  "  --debug-log                    debug test logging output\n"

Hm, documentiong this option is at least questionable...

>+                  "  -k, --keep-going               gtester-specific argument\n"

The docs for -k should be removed, it's really only supported by gtester, not the tests themselves (because resuming a failing test requires a new fork).

>+                  "  --GTestLogFD=N                 gtester-specific argument\n"
>+                  "  --GTestSkipCount=N             gtester-specific argument\n",

The docs for these two options also should be removed, they are not part of our officially supproted command line interface for test programs (i.e. implementation details).

The most important docs for unit tests are actually missing, here're the relevant snippets from gtester -h that are also supported by individual unit tests:
Options:
  -h, --help                  show this help message
  -l                          list paths of available test cases
  -m=perf, -m=slow, -m=quick -m=thorough
                              run test cases in mode perf, slow/thorough or quick (default)
  -p=TESTPATH                 only start test cases matching TESTPATH
  --seed=SEEDSTRING           start all tests with random number seed SEEDSTRING
  -o=LOGFILE                  write the test log to LOGFILE
  -q, --quiet                 suppress per test output
  --verbose                   report success per testcase

>+                  argv[0]);
>+          exit (0);
>+        }
>     }
>   /* collapse argv */
>   e = 1;
Comment 8 Christian Dywan 2008-10-17 11:56:01 UTC
(In reply to comment #7)
> (From update of attachment 117842 [details] [review] [edit])
> Only saw this now and have a few review bits still:
> 
> >diff --git a/glib/gtestutils.c b/glib/gtestutils.c
> >index 8c0eb1f..0b77908 100644
> >--- a/glib/gtestutils.c
> >+++ b/glib/gtestutils.c
> >@@ -347,6 +347,27 @@ parse_args (gint    *argc_p,
> >             }
> >           argv[i] = NULL;
> >         }
> >+      else if (strcmp ("-?", argv[i]) == 0 || strcmp ("--help", argv[i]) == 0)
> 
> This should definitely support '-h' as --help alias. We do that everywhere
> else and '-h' is a widely used abbreviatiuon for --help.

This actually led me to file bug 556706 since unfortunately we do *not* currently do the same help switches, although -h is supported by more Gtk+ tools than -?.
In the bug I suggested to advertise -h but also support -?. Thus I shall do so in this bug as well - depending on whether the proposal in bug 556706 is accepted.

> >+        {
> >+          printf ("Usage:\n"
> >+                  "  %s [OPTION...]\n\n"
> >+                  "Help Options:\n"
> >+                  "  -?, --help                     Show help options\n"
> >+                  "Test Options:\n"
> >+                  "  -l                             List test cases
> > available in a test executable\n"
> >+                  "  -seed=RANDOMSEED               Provide a random seed
> > to reproduce test\n"
> 
> The docs from gtester here read:
>   --seed=SEEDSTRING           start all tests with random number seed
> SEEDSTRING
> Would be nice to keep them consistent.

Note that gtestutils already has documentation and that's what I based the output on. I agree, it should be fixed either way.

> >+                  "                                 runs using random numbers\n"
> >+                  "  --verbose                      Run tests verbosely\n"
> >+                  "  -q, --quiet                    Run tests quietly\n"
> >+                  "  -p TESTPATH                    execute all tests matching TESTPATH\n"
> >+                  "  -m {perf|slow|thorough|quick}  Execute tests according modes\n"
> >+                  "  --debug-log                    debug test logging output\n"
> 
> Hm, documentiong this option is at least questionable...
> 
> >+                  "  -k, --keep-going               gtester-specific argument\n"
> 
> The docs for -k should be removed, it's really only supported by gtester, not
> the tests themselves (because resuming a failing test requires a new fork).
> 
> >+                  "  --GTestLogFD=N                 gtester-specific argument\n"
> >+                  "  --GTestSkipCount=N             gtester-specific argument\n",
> 
> The docs for these two options also should be removed, they are not part
> of our officially supproted command line interface for test programs (i.e.
> implementation details).

Well, they are documented in the reference.

I conclude that those options should not be in the reference either. It does make sense to me, from the nature of the options.

> The most important docs for unit tests are actually missing, here're the
> relevant snippets from gtester -h that are also supported by individual unit
> tests:
> Options:
>   -h, --help                  show this help message
>   -l                          list paths of available test cases
>   -m=perf, -m=slow, -m=quick -m=thorough
>                               run test cases in mode perf, slow/thorough or
> quick (default)
>   -p=TESTPATH                 only start test cases matching TESTPATH
>   --seed=SEEDSTRING           start all tests with random number seed
> SEEDSTRING
>   -o=LOGFILE                  write the test log to LOGFILE
>   -q, --quiet                 suppress per test output
>   --verbose                   report success per testcase

From what I can see, only these two are missing:

> --g-fatal-warnings              make warnings fatal (abort)

I agree, it should be added, I overlooked that.

> -o=LOGFILE                      write the test log to LOGFILE

That's not supported by g_test_init at all. If you think it should be, please file a bug ^_^

My verdict is, the output provided by g_test_init should be improved and after that I suggest to apply according changes to gtester, so that we have the same wording.
Comment 9 Tim Janik 2008-10-17 12:32:47 UTC
(In reply to comment #8)
> > >+      else if (strcmp ("-?", argv[i]) == 0 || strcmp ("--help", argv[i]) == 0)
> > 
> > This should definitely support '-h' as --help alias. We do that everywhere
> > else and '-h' is a widely used abbreviatiuon for --help.
> 
> This actually led me to file bug 556706 since unfortunately we do *not*
> currently do the same help switches, although -h is supported by more Gtk+
> tools than -?.

Ok, thanks. Let's not block this bug on the --help issue though, the remaining things here can still be fixed.

> > >+        {
> > >+          printf ("Usage:\n"
> > >+                  "  %s [OPTION...]\n\n"
> > >+                  "Help Options:\n"
> > >+                  "  -?, --help                     Show help options\n"
> > >+                  "Test Options:\n"
> > >+                  "  -l                             List test cases
> > > available in a test executable\n"
> > >+                  "  -seed=RANDOMSEED               Provide a random seed
> > > to reproduce test\n"
> > 
> > The docs from gtester here read:
> >   --seed=SEEDSTRING           start all tests with random number seed
> > SEEDSTRING
> > Would be nice to keep them consistent.
> 
> Note that gtestutils already has documentation and that's what I based the
> output on. I agree, it should be fixed either way.

gtester is the canonical reference if in doubt ;)
You're right about the reference needing adoptions as well then, please include that in your patch.

> > >+                  "  --GTestLogFD=N                 gtester-specific argument\n"
> > >+                  "  --GTestSkipCount=N             gtester-specific argument\n",
> > 
> > The docs for these two options also should be removed, they are not part
> > of our officially supproted command line interface for test programs (i.e.
> > implementation details).
> 
> Well, they are documented in the reference.

Again, the reference needs fixing as well.

> I conclude that those options should not be in the reference either. It does
> make sense to me, from the nature of the options.

I guess the reference *could* mention those including -k if it made clear that those are actually internal/reserved.

> > The most important docs for unit tests are actually missing, here're the
> > relevant snippets from gtester -h that are also supported by individual unit
> > tests:
> > Options:
> >   -h, --help                  show this help message
> >   -l                          list paths of available test cases
> >   -m=perf, -m=slow, -m=quick -m=thorough
> >                               run test cases in mode perf, slow/thorough or
> > quick (default)
> >   -p=TESTPATH                 only start test cases matching TESTPATH
> >   --seed=SEEDSTRING           start all tests with random number seed
> > SEEDSTRING
> >   -o=LOGFILE                  write the test log to LOGFILE
> >   -q, --quiet                 suppress per test output
> >   --verbose                   report success per testcase

> > -o=LOGFILE                      write the test log to LOGFILE
> 
> That's not supported by g_test_init at all. If you think it should be, please
> file a bug ^_^

Sorry, "-o" is bogus, it's a c'n'p error i frogot to remove.

> My verdict is, the output provided by g_test_init should be improved and after
> that I suggest to apply according changes to gtester, so that we have the same
> wording.

As mentioned above, gtester should be considered the canonical reference (except for gtester-only options of course), the other doc snippets were contributed or added after the fact, so need fixing.
Comment 10 Christian Dywan 2008-10-17 13:35:04 UTC
Created attachment 120780 [details] [review]
Improve help output

(In reply to comment #9)
> (In reply to comment #8)
> > > >+      else if (strcmp ("-?", argv[i]) == 0 || strcmp ("--help", argv[i]) == 0)
> > > 
> > > This should definitely support '-h' as --help alias. We do that everywhere
> > > else and '-h' is a widely used abbreviatiuon for --help.
> > 
> > This actually led me to file bug 556706 since unfortunately we do *not*
> > currently do the same help switches, although -h is supported by more Gtk+
> > tools than -?.
> 
> Ok, thanks. Let's not block this bug on the --help issue though, the remaining
> things here can still be fixed.

Okay, so here is a patch which does the following:

- Accept -h and -?, help output shows '-h, --help'.
- Adapt the wording except for "-m" since the gtestutil version is much nicer
- Change the format slightly to look like GOptionContext output
- Remove -k and and the two internal options
- Removed the documentation for those options. I think it doesn't seem all that useful if you practically need the source to understand them.
- Actually moved the options around to match getesters order

- "Start all tests with random number seed SEEDSTRING"
+ "Start all tests with random seed SEEDSTRING"

I actually left out the 'number' because it doesn't fit my 80 column console otherwise. I think it's still clear enough.

If you are fine with those changes I'd like to adjust gtester afterwards with regard to the remaining differences.
Comment 11 Christian Dywan 2008-10-17 17:36:24 UTC
Created attachment 120796 [details] [review]
Adkjust GTester help format, recognize -?

This adjusts GTester according to the previous g-test-utils change.
Comment 12 Bastien Nocera 2009-03-23 14:31:10 UTC
(In reply to comment #3)
> (In reply to comment #2)
> > I'd much rather accept a patch to make use of the option parser that lives next
> > file in the same library. Really, this should not have been accepted in this
> > form...
> 
> You mean using GOptionContext. In fact I wanted to do that originally as you
> can see in the bug description, but, which I should have mentioned, I was
> pointed out while writing the patch, that to be able to actually test
> GOptionContext it cannot be used in g_test_init. ie. the code should rely on as
> few functionality as possible.

This clearly sucks for libraries that live on top of glib, and want to use GTester. Just like we have gtk_init() and gtk_get_option_group(), we could have a g_test_init() and a g_test_get_option_group().
Comment 13 Christian Dywan 2009-09-04 12:08:23 UTC
ping
Comment 14 Matthias Clasen 2012-08-17 01:33:01 UTC
Pushed a similar patch