[PATCH] Fix xmlout option

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fix xmlout option

Omair Majid
Hi,

The attached patch fixes the issue in mauve when running the entire test
suite with -xmlout fails:

$ java Harness -vm /usr/bin/java -xmlout xmloutputfile
No tests were run.  Possible reasons may be listed below.
Did you specify a test that doesn't exist or a folder that contains no
tests?

The bug seems to be that mauve ignores the -xmlout option and then
treats xmloutputfile as the name of the test to run. The patch adds a
case in Harness.setupHarness() so that the argument after -xmlout is
skipped (it is later checked in RunnerProcess).

Cheers,
Omair

--- Harness.java.orig 2009-02-12 11:30:11.000000000 -0500
+++ Harness.java 2009-02-12 11:34:23.000000000 -0500
@@ -338,6 +338,13 @@
                     "after '-timeout'.  Exit");
             runner_timeout = Long.parseLong(args[i]);
           }
+        else if (args[i].equals("-xmlout"))
+          {
+            // Option handled by RunnerProcess
+            // Next arg is filename; RunnerProcess checks the args
+            // again
+            ++i;
+          }
         else if (args[i].charAt(0) == '-')
           {
             // One of the ignored options (handled by RunnerProcess)
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix xmlout option

Mark Wielaard
Hi Omair,

On Thu, 2009-02-12 at 11:51 -0500, Omair Majid wrote:

> The attached patch fixes the issue in mauve when running the entire test
> suite with -xmlout fails:
>
> $ java Harness -vm /usr/bin/java -xmlout xmloutputfile
> No tests were run.  Possible reasons may be listed below.
> Did you specify a test that doesn't exist or a folder that contains no
> tests?
>
> The bug seems to be that mauve ignores the -xmlout option and then
> treats xmloutputfile as the name of the test to run. The patch adds a
> case in Harness.setupHarness() so that the argument after -xmlout is
> skipped (it is later checked in RunnerProcess).

Thanks. It took me some time to understand how this was supposed to work
with the ping-pong between Harness and RunnerProcess. I am afraid you
might be the first one in a long time that has worked with the xml
output. So don't be surprised if you find more bugs. Sorry about that.

> --- Harness.java.orig 2009-02-12 11:30:11.000000000 -0500
> +++ Harness.java 2009-02-12 11:34:23.000000000 -0500
> @@ -338,6 +338,13 @@
>                      "after '-timeout'.  Exit");
>              runner_timeout = Long.parseLong(args[i]);
>            }
> +        else if (args[i].equals("-xmlout"))
> +          {
> +            // Option handled by RunnerProcess
> +            // Next arg is filename; RunnerProcess checks the args
> +            // again
> +            ++i;
> +          }

I think you should explicitly check that args.length still has another
element like the other checks before this one. The help message implies
that the file name is optional, but it really isn't since RunnerProcess
depends on it. Could you make that change and update the help message?

Thanks,

Mark

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix xmlout option

Omair Majid
Hi Mark,

Mark Wielaard wrote:
> Thanks. It took me some time to understand how this was supposed to work
> with the ping-pong between Harness and RunnerProcess. I am afraid you
> might be the first one in a long time that has worked with the xml
> output. So don't be surprised if you find more bugs. Sorry about that.

I am just glad there is an xmlout option at all!

> I think you should explicitly check that args.length still has another
> element like the other checks before this one. The help message implies
> that the file name is optional, but it really isn't since RunnerProcess
> depends on it. Could you make that change and update the help message?

Good catch. Fixed patch attached. I am not sure how the help message
indicates that the file name is optional. 'filename' in '-file
[filename]' is required and so is 'millis' in '-timeout [millis]'.

Cheers,
Omair



--- Harness.java.orig 2009-02-12 11:30:11.000000000 -0500
+++ Harness.java 2009-02-13 09:58:05.000000000 -0500
@@ -338,6 +338,14 @@
                     "after '-timeout'.  Exit");
             runner_timeout = Long.parseLong(args[i]);
           }
+        else if (args[i].equals("-xmlout"))
+          {
+            // User wants output in an xml file
+            if (++i >= args.length)
+              throw new RuntimeException ("No file " +
+                    "given after '-xmlout'.  Exit");
+            // the filename is used directly from args
+          }
         else if (args[i].charAt(0) == '-')
           {
             // One of the ignored options (handled by RunnerProcess)
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fix xmlout option

Mark Wielaard
Hi Omair,

On Fri, 2009-02-13 at 10:07 -0500, Omair Majid wrote:
> Good catch. Fixed patch attached. I am not sure how the help message
> indicates that the file name is optional. 'filename' in '-file
> [filename]' is required and so is 'millis' in '-timeout [millis]'.

O right, hmmm, how unconventional. But ok, lets keep them all the same.
Applied your patch as:

2009-02-13  Omair Majid  <[hidden email]>

        * Harness.java (setupHarness): Check for -xmlout file argument.

Thanks,

Mark