[PATCH] Fix xmlout option

[PATCH] Fix xmlout option

[Omair Majid]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

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

[-- Attachment #2: harness-option-xmlout.patch --]
[-- Type: text/plain, Size: 603 bytes --]

--- 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)

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

Re: [PATCH] Fix xmlout option

[Omair Majid]

[-- Attachment #1: Type: text/plain, Size: 884 bytes --]

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



[-- Attachment #2: harness-option-xmlout.patch --]
[-- Type: text/plain, Size: 699 bytes --]

--- 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)

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  <omajid@redhat.com>

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

Thanks,

Mark