You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Kamesh Jayachandran <ka...@collab.net> on 2006/04/05 19:49:50 UTC
[PATCH] #2532 Make svn C test harness to behave the same way as python
test harness w.r.t --fs-type, --list switches.
Hi All,
Find the attached patch.
With regards
Kamesh Jayachandran
[[[
Fix issue #2532, Make svn C test harness to behave the same way as
python test harness w.r.t --fs-type, --list switches.
* subversion/tests/svn_test_main.c
(main): Using apr_getopt to parse the command line switches.
Thanks to apr_getopt now the svn 'C' test programs,
can be called with --fs-type [bdb/fsfs].
The --list switch has been implemented to list the tests.
Removed the notice message printed, on non-numeric arguments
beyond argv[1].
]]]
Re: [PATCH] #2532 Make svn C test harness to behave the same way
as python test harness w.r.t --fs-type, --list switches.
Posted by "C. Michael Pilato" <cm...@collab.net>.
C. Michael Pilato wrote:
> I'll make these final tweaks myself and commit up your otherwise
> well-written patch after testing it (which I'm doing right now).
Had to correct a few warnings ('const' not present on some things), but
other than that, the patch worked as I expected it to. r19197. Thanks, Kamesh.
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] #2532 Make svn C test harness to behave the same way
as python test harness w.r.t --fs-type, --list switches.
Posted by "C. Michael Pilato" <cm...@collab.net>.
Kamesh Jayachandran wrote:
> Thanks for the comments Garrett.
>
> Attaching the new patch.
>
> With regards
> Kamesh Jayachandran
>
> [[[
> Fix issue #2532, Make svn C test harness to behave the same way as
> python test harness w.r.t --fs-type, --list switches.
>
> * subversion/tests/svn_test_main.c
> (main): Using apr_getopt to parse the command line switches.
> Thanks to apr_getopt now the svn 'C' test programs,
> can be called with --fs-type [bdb/fsfs].
> The --list switch has been implemented to list the tests.
> Removed the notice message printed, on non-numeric arguments
> beyond argv[1].
> ]]]
[...]
> Index: subversion/tests/svn_test_main.c
> ===================================================================
> --- subversion/tests/svn_test_main.c (revision 19185)
> +++ subversion/tests/svn_test_main.c (working copy)
> @@ -26,6 +26,8 @@
> #include <apr_general.h>
> #include <apr_lib.h>
>
> +#include "svn_cmdline.h"
> +#include "svn_opt.h"
> #include "svn_pools.h"
> #include "svn_error.h"
> #include "svn_test.h"
> @@ -46,7 +48,25 @@
> /* Test option: Remove test directories after success */
> static int cleanup_mode = 0;
>
> +enum {
> + cleanup_opt = SVN_OPT_FIRST_LONGOPT_ID,
> + fstype_opt,
> + list_opt,
> + verbose_opt
> +};
>
> +static const apr_getopt_option_t cl_options[] =
> +{
> + {"cleanup", cleanup_opt, 0,
> + N_("remove test directories after success")},
> + {"fs-type", fstype_opt, 1,
> + N_("specify a fs-type ARG")},
I think I'd prefer "specify a filesystem backend type ARG" here. If the
Python tests use the abbreviation "fs-type" to describe the "fs-type"
option, they too should be fixed (though that can be seen as outside the
scope of this patch).
> + switch (opt_id) {
> + case cleanup_opt:
> + cleanup_mode = 1;
> + break;
> + case fstype_opt:
> + opts.fs_type = apr_pstrdup(pool, opt_arg);
> + break;
> + case list_opt:
> + list_mode = 1;
> + break;
Oops -- a little bit of odd indentation for that "break;" statement.
I'll make these final tweaks myself and commit up your otherwise
well-written patch after testing it (which I'm doing right now).
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Re: [PATCH] #2532 Make svn C test harness to behave the same way
as python test harness w.r.t --fs-type, --list switches.
Posted by Kamesh Jayachandran <ka...@collab.net>.
Thanks for the comments Garrett.
Attaching the new patch.
With regards
Kamesh Jayachandran
[[[
Fix issue #2532, Make svn C test harness to behave the same way as
python test harness w.r.t --fs-type, --list switches.
* subversion/tests/svn_test_main.c
(main): Using apr_getopt to parse the command line switches.
Thanks to apr_getopt now the svn 'C' test programs,
can be called with --fs-type [bdb/fsfs].
The --list switch has been implemented to list the tests.
Removed the notice message printed, on non-numeric arguments
beyond argv[1].
]]]
Garrett Rooney wrote:
> On 4/5/06, Kamesh Jayachandran <ka...@collab.net> wrote:
>
> Thanks for the patch! A few comments:
>
>
>> Index: subversion/tests/svn_test_main.c
>> ===================================================================
>> --- subversion/tests/svn_test_main.c (revision 19185)
>> +++ subversion/tests/svn_test_main.c (working copy)
>> @@ -26,6 +26,8 @@
>> #include <apr_general.h>
>> #include <apr_lib.h>
>>
>> +#include "svn_cmdline.h"
>> +#include "svn_opt.h"
>> #include "svn_pools.h"
>> #include "svn_error.h"
>> #include "svn_test.h"
>> @@ -46,7 +48,25 @@
>> /* Test option: Remove test directories after success */
>> static int cleanup_mode = 0;
>>
>> +typedef enum {
>> + cleanup_opt = SVN_OPT_FIRST_LONGOPT_ID,
>> + fstype_opt,
>> + list_opt,
>> + verbose_opt
>> +};
>>
>
> If you're going to typedef it, you need to typedef it to something.
> So either drop the typedef, or give it a name after the }.
>
>
>> +const apr_getopt_option_t cl_options[] =
>> +{
>> + {"cleanup", cleanup_opt, 0,
>> + N_("remove test directories after success")},
>> + {"fs-type", fstype_opt, 1,
>> + N_("specify a fs-type ARG")},
>> + {"list", list_opt, 0,
>> + N_("lists all the tests with their short description")},
>> + {"verbose", verbose_opt, 0,
>> + N_("print extra information")},
>> + {0, 0, 0, 0}
>> +};
>>
>
> This should be static.
>
>
>> /* ================================================================= */
>> /* Stuff for cleanup processing */
>>
>> @@ -208,6 +228,11 @@
>> apr_pool_t *pool, *test_pool;
>> int ran_a_test = 0;
>> char **arg;
>> + int list_mode = 0;
>> + int opt_id;
>> + apr_status_t apr_err;
>> + apr_getopt_t *os;
>> + svn_error_t *err;
>> /* How many tests are there? */
>> int array_size = get_array_size();
>>
>> @@ -244,15 +269,37 @@
>> test_argc = argc;
>> test_argv = argv;
>>
>> - /* Scan the command line for the --verbose and --cleanup flags */
>> - for (arg = &argv[1]; *arg; ++arg)
>> + err = svn_cmdline__getopt_init(&os, pool, argc, argv);
>> + if (err)
>> + return svn_cmdline_handle_exit_error(err, pool, prog_name);
>> + while (1)
>> {
>> - if (strcmp(*arg, "--cleanup") == 0)
>> - cleanup_mode = 1;
>> - else if (strcmp(*arg, "--verbose") == 0)
>> - verbose_mode = 1;
>> - else if (strncmp(*arg, "--fs-type=", 10) == 0)
>> - opts.fs_type = apr_pstrdup(pool, (*arg) + 10);
>> + const char *opt_arg;
>> +
>> + /* Parse the next option. */
>> + apr_err = apr_getopt_long(os, cl_options, &opt_id, &opt_arg);
>> + if (APR_STATUS_IS_EOF(apr_err))
>> + break;
>> + else if (apr_err)
>> + {
>> + printf("apr_getopt_long() failed.\n");
>> + exit(1);
>>
>
> Why did it fail? Perhaps apr_strerror would be useful here.
>
>
>> + }
>> +
>> + switch (opt_id) {
>> + case cleanup_opt:
>> + cleanup_mode = 1;
>> + break;
>> + case fstype_opt:
>> + opts.fs_type= apr_pstrdup(pool, opt_arg);
>>
>
> Need a space before the =.
>
>
>> + break;
>> + case list_opt:
>> + list_mode = 1;
>> + break;
>> + case verbose_opt:
>> + verbose_mode = 1;
>> + break;
>> + }
>> }
>>
>> /* Create an iteration pool for the tests */
>> @@ -261,7 +308,7 @@
>>
>> if (argc >= 2) /* notice command-line arguments */
>> {
>> - if (! strcmp(argv[1], "list"))
>> + if (! strcmp(argv[1], "list") || list_mode)
>> {
>> ran_a_test = 1;
>>
>> @@ -294,11 +341,6 @@
>> svn_pool_clear(test_pool);
>> svn_pool_clear(cleanup_pool);
>> }
>> - else if (argv[i][0] != '-')
>> - {
>> - /* (probably) a source directory pathname */
>> - printf("notice: ignoring argument %d: '%s'\n", i, argv[i]);
>> - }
>> }
>> }
>> }
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
>> For additional commands, e-mail: dev-help@subversion.tigris.org
>>
>>
>>
>
>
Re: [PATCH] #2532 Make svn C test harness to behave the same way as python test harness w.r.t --fs-type, --list switches.
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 4/5/06, Kamesh Jayachandran <ka...@collab.net> wrote:
Thanks for the patch! A few comments:
>
> Index: subversion/tests/svn_test_main.c
> ===================================================================
> --- subversion/tests/svn_test_main.c (revision 19185)
> +++ subversion/tests/svn_test_main.c (working copy)
> @@ -26,6 +26,8 @@
> #include <apr_general.h>
> #include <apr_lib.h>
>
> +#include "svn_cmdline.h"
> +#include "svn_opt.h"
> #include "svn_pools.h"
> #include "svn_error.h"
> #include "svn_test.h"
> @@ -46,7 +48,25 @@
> /* Test option: Remove test directories after success */
> static int cleanup_mode = 0;
>
> +typedef enum {
> + cleanup_opt = SVN_OPT_FIRST_LONGOPT_ID,
> + fstype_opt,
> + list_opt,
> + verbose_opt
> +};
If you're going to typedef it, you need to typedef it to something.
So either drop the typedef, or give it a name after the }.
> +const apr_getopt_option_t cl_options[] =
> +{
> + {"cleanup", cleanup_opt, 0,
> + N_("remove test directories after success")},
> + {"fs-type", fstype_opt, 1,
> + N_("specify a fs-type ARG")},
> + {"list", list_opt, 0,
> + N_("lists all the tests with their short description")},
> + {"verbose", verbose_opt, 0,
> + N_("print extra information")},
> + {0, 0, 0, 0}
> +};
This should be static.
> /* ================================================================= */
> /* Stuff for cleanup processing */
>
> @@ -208,6 +228,11 @@
> apr_pool_t *pool, *test_pool;
> int ran_a_test = 0;
> char **arg;
> + int list_mode = 0;
> + int opt_id;
> + apr_status_t apr_err;
> + apr_getopt_t *os;
> + svn_error_t *err;
> /* How many tests are there? */
> int array_size = get_array_size();
>
> @@ -244,15 +269,37 @@
> test_argc = argc;
> test_argv = argv;
>
> - /* Scan the command line for the --verbose and --cleanup flags */
> - for (arg = &argv[1]; *arg; ++arg)
> + err = svn_cmdline__getopt_init(&os, pool, argc, argv);
> + if (err)
> + return svn_cmdline_handle_exit_error(err, pool, prog_name);
> + while (1)
> {
> - if (strcmp(*arg, "--cleanup") == 0)
> - cleanup_mode = 1;
> - else if (strcmp(*arg, "--verbose") == 0)
> - verbose_mode = 1;
> - else if (strncmp(*arg, "--fs-type=", 10) == 0)
> - opts.fs_type = apr_pstrdup(pool, (*arg) + 10);
> + const char *opt_arg;
> +
> + /* Parse the next option. */
> + apr_err = apr_getopt_long(os, cl_options, &opt_id, &opt_arg);
> + if (APR_STATUS_IS_EOF(apr_err))
> + break;
> + else if (apr_err)
> + {
> + printf("apr_getopt_long() failed.\n");
> + exit(1);
Why did it fail? Perhaps apr_strerror would be useful here.
> + }
> +
> + switch (opt_id) {
> + case cleanup_opt:
> + cleanup_mode = 1;
> + break;
> + case fstype_opt:
> + opts.fs_type= apr_pstrdup(pool, opt_arg);
Need a space before the =.
> + break;
> + case list_opt:
> + list_mode = 1;
> + break;
> + case verbose_opt:
> + verbose_mode = 1;
> + break;
> + }
> }
>
> /* Create an iteration pool for the tests */
> @@ -261,7 +308,7 @@
>
> if (argc >= 2) /* notice command-line arguments */
> {
> - if (! strcmp(argv[1], "list"))
> + if (! strcmp(argv[1], "list") || list_mode)
> {
> ran_a_test = 1;
>
> @@ -294,11 +341,6 @@
> svn_pool_clear(test_pool);
> svn_pool_clear(cleanup_pool);
> }
> - else if (argv[i][0] != '-')
> - {
> - /* (probably) a source directory pathname */
> - printf("notice: ignoring argument %d: '%s'\n", i, argv[i]);
> - }
> }
> }
> }
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org