Mailing-List: contact commits-help@harmony.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@harmony.apache.org
Message-ID: <7688052.431311282168157509.JavaMail.jira@thor>
Date: Wed, 18 Aug 2010 17:49:17 -0400 (EDT)
From: "Mark Hindess (JIRA)" <jira@apache.org>
To: commits@harmony.apache.org
Subject: [jira] Commented: (HARMONY-6620) [jdktools] Unit tests to test the
 functionality of the javac/javaw binaries
In-Reply-To: <3423419.311821281641058535.JavaMail.jira@thor>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


    [ https://issues.apache.org/jira/browse/HARMONY-6620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12900049#action_12900049 ] 

Mark Hindess commented on HARMONY-6620:
---------------------------------------

You can either use the exclude lists - i.e. add the javaw test name to exclude files for non-windows platforms - or split the test sources in to common, unix, windows directories as has been done in for example classlib/modules/{auth,misc,nio}.

To avoid going around this loop to many times I've reviewed the javac test in more detail. Some comments in no particular order.

These tests do not pass on linux (or windows I'd assume) with the
federated build.

The test_nonExists method asserts that the error output contains the
string "not found", but the output actually says:

  File no_this_test.java is missing

Checking the ecj jar in the federated build I find:

  bash$ unzip -p target/hdk/jdk/lib/ecj-3.5.1.jar |grep 'not found'
  bash$ unzip -p target/hdk/jdk/lib/ecj-3.5.1.jar |grep 'is missing'
  Binary file (standard input) matches

so I'm not sure where the "not found" message you are testing for
might be coming from?

Why is errOutput returned from runExe as a StringBuilder?  I may be
missing something but it looks like the callers only use
errOutput.toString() - currently several times - when you could just
return the string.

getProcessOutput should take Process as the first argument not the
Object since it only uses the Process element of the Object array.

Since all runExe callers call getProcessOutput why not have runExe
take a displayOutput boolean and then return:

 { proc, getProcessOutput(proc, displayOutput), errBuf.toString() }

(Perhaps proc is no longer needed but perhaps you might also want to
check the exit status on some tools?)  I'd be a little tempted to
trim the error output string too.

Can you break up the assertTrue asserts?  What you have at the moment
is the junit equivalent of:

  if (A || B) {
    System.err.println("A or B is wrong");
    ...                          
  }

which is a pet hate of mine.  For example (assuming errOutput is now
a string for brevity!):

  assertTrue("The output should have " + testStr + " Error - " + errOutput,
             errOutput.contains(testStr) && toString().contains("is missing") );

should become:

  assertTrue("The output should contain \"" + testStr + "\" Error - \"" 
             + errOutput + "\"",
             errOutput.contains(testStr));
  assertTrue("The output should contain \"is missing\" Error - \"" 
             + errOutput + "\"",
             errOutput.contains("is missing"));

ditto for several other asserts.

It might be better to consistently name variables such as "stdOut" and
"errOutput".  Those names seem rather confused.  Perhaps "stdOut" and
"stdErr"?

Typical style for Harmony code tends to have the opening curly bracket
of a method on the end of the line not on a line on its own.

Rather than use a displayOutput boolean, it might be better to replace
the asserts with something more verbose such as:

    if (!errOutput.contains(testStr)) {
        System.err.println("Output: " + stdOut);
        System.err.println("Errors: " + errOutput);
        fail("The output should contain \"" + testStr + "\"");
    }

so that if/when the test fails we get the information required to
debug the failure - rather than having to rebuild the test.

What is the benefit of writing the tests as:

    final String srcFile =  RESOURCES + "Simple.java";
    final File f = new File(srcFile);
    final String testStr =  f.getAbsolutePath();

rather than:

    final String testStr = RESOURCES + "Simple.java";

That is, don't create File objects and just use relative paths?
Similarly for things like:

    final String classFile = f.getParent() + "\\Sample.class";
    final File clsFile = new File(classFile);

which only works on windows - because of the '\' char - rather than:

    final File clsFile = new File(RESOURCES + "Sample.class");

Please can you use four spaces rather than tabs as is typical
Harmony style.


> [jdktools] Unit tests to test the functionality of the javac/javaw binaries
> ---------------------------------------------------------------------------
>
>                 Key: HARMONY-6620
>                 URL: https://issues.apache.org/jira/browse/HARMONY-6620
>             Project: Harmony
>          Issue Type: Test
>          Components: JDK
>         Environment: Windows and Linux
>            Reporter: Prashanth KS
>             Fix For: 5.0M15
>
>         Attachments: 001_HARMONY_6620.patch, 002_HARMONY_6620.patch, resources.zip
>
>
> The ANT unit tests written for the jdktools binaries such as javac aren't flexible enough. i.e. if there is a test failure then the rest of the tests are skipped. Moreover, there aren't a lot of test scenarios created for these binaries.
> Added Unit tests to test the javac.exe and javaw.exe tools. This helps in providing a better analysis of these tools across different runtimes.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.