On Wed, Jul 23, 2014 at 02:13:14PM +0200, Daniel Vetter wrote:
> On Wed, Jul 23, 2014 at 11:57:50AM +0100, Thomas Wood wrote:
> > Move option parsing into common_init so it can be shared between simple
> > tests and tests with subtests. This allows for more common command line
> > behaviour across all tests.
> >
> > Signed-off-by: Thomas Wood <thomas.wood@xxxxxxxxx>
> > ---
> > lib/igt_core.c | 118 ++++++++++++++++++++++++++++++++++++---------------------
> > 1 file changed, 74 insertions(+), 44 deletions(-)
> >
> > diff --git a/lib/igt_core.c b/lib/igt_core.c
> > index 5c20581..ee6f90c 100644
> > --- a/lib/igt_core.c
> > +++ b/lib/igt_core.c
> > @@ -291,30 +291,11 @@ static void oom_adjust_for_doom(void)
> > igt_assert(write(fd, always_kill, sizeof(always_kill)) == sizeof(always_kill));
> > }
> >
> > -/**
> > - * igt_subtest_init_parse_opts:
> > - * @argc: argc from the test's main()
> > - * @argv: argv from the test's main()
> > - * @extra_short_opts: getopt_long() compliant list with additional short options
> > - * @extra_long_opts: getopt_long() compliant list with additional long options
> > - * @help_str: help string for the additional options
> > - * @extra_opt_handler: handler for the additional options
> > - *
> > - * This function handles the subtest related cmdline options and allows an
> > - * arbitrary set of additional options. This is useful for tests which have
> > - * additional knobs to tune when run manually like the number of rounds execute
> > - * or the size of the allocated buffer objects.
> > - *
> > - * Tests without special needs should just use igt_subtest_init() or use
> > - * #igt_main directly instead of their own main() function.
> > - *
> > - * Returns: Forwards any option parsing errors from getopt_long.
> > - */
> > -int igt_subtest_init_parse_opts(int argc, char **argv,
> > - const char *extra_short_opts,
> > - struct option *extra_long_opts,
> > - const char *help_str,
> > - igt_opt_handler_t extra_opt_handler)
> > +static int common_init(int argc, char **argv,
> > + const char *extra_short_opts,
> > + struct option *extra_long_opts,
> > + const char *help_str,
> > + igt_opt_handler_t extra_opt_handler)
> > {
> > int c, option_index = 0;
> > static struct option long_options[] = {
> > @@ -328,8 +309,18 @@ int igt_subtest_init_parse_opts(int argc, char **argv,
> > int extra_opt_count;
> > int all_opt_count;
> > int ret = 0;
> > + char *env = getenv("IGT_LOG_LEVEL");
> >
> > - test_with_subtests = true;
> > + if (env) {
> > + if (strcmp(env, "debug") == 0)
> > + igt_log_level = IGT_LOG_DEBUG;
> > + else if (strcmp(env, "info") == 0)
> > + igt_log_level = IGT_LOG_INFO;
> > + else if (strcmp(env, "warn") == 0)
> > + igt_log_level = IGT_LOG_WARN;
> > + else if (strcmp(env, "none") == 0)
> > + igt_log_level = IGT_LOG_NONE;
> > + }
> >
> > command_str = argv[0];
> > if (strrchr(command_str, '/'))
> > @@ -389,36 +380,70 @@ int igt_subtest_init_parse_opts(int argc, char **argv,
> > }
> > }
> >
> > - igt_install_exit_handler(check_igt_exit);
> > oom_adjust_for_doom();
> >
> > out:
> > free(short_opts);
> > free(combined_opts);
> > +
> > + /* exit immediately if this test has no subtests and a subtest or the
> > + * list of subtests has been requested */
> > + if (!test_with_subtests) {
> > + if (run_single_subtest) {
> > + igt_warn("Unknown subtest: %s\n", run_single_subtest);
> > + exit(-1);
> > + }
> > + if (list_subtests)
> > + exit(-1);
>
> Instead of -1 I think we should have a common IGT_EXIT_INVALID code like
> 79 similar to skip and timeout and use this one here and in the code
> you've added to igt_exit in a previous patch.
>
> Probably easiest if you do this as a fixup on top.
>
> Otherwise I think this goes into the right direction, so ack on the entire
> series. I haven't done an in-depth review, but didn't spot anything else
> really.
Aside: If we have this special exit code which no other tests uses
otherwise we can catch it in piglit. That will be extremely useful to
catch all those offenders who accidentally left a printf or similar
unguarded by igt_fixture or igt_subtest somewhere in their code.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
