RFR 8066085: Need a sanity test for rmic -iiop
felix.yang at oracle.com
Fri Dec 19 09:56:13 UTC 2014
please review the updated webrev:
On 12/12/2014 4:08 PM, Stuart Marks wrote:
> On 12/11/14 8:41 PM, FELIX YANG wrote:
>> Hi all,
>> please review the fix to add a sanity checking for rmic -iiop
> Hi Felix,
> Thanks for picking up the writing of this test. I have several comments.
> 1. Usually we try to avoid naming tests after bug IDs. There are such
> tests in the suite but most tests have more descriptive names. I'd
> suggest something like "iiopSanityTest".
> 2. There are other rmic tests in jdk/test/sun/rmi/rmic; the test
> should be moved there (and the path to the TestLibrary updated
> 3. The test summary should be a bit more explicit. Instead of "a
> sanity test for rmic -iiop compiling" I'd suggest something like
> "Compiles a PortableRemoteObject with rmic -iiop and ensures that stub
> and tie classes are generated."
> 4. When invoking rmic, you need to make sure to pass the -iiop option.
> 5. Add check for the _Tie class as well as the _Stub class. Not that
> it really matters, since we know these classes are in the unnamed
> package, but the stub and tie class files would end up in directories
> corresponding to their package names -- if they were in a named package.
> 6. In doTest(), the try/catch of InterruptedException and IOException
> and call to TestLibrary.bomb() is mostly unnecessary. (Yes, other RMI
> tests do this, and I've been removing it where appropriate. In the RMI
> test library, the bomb() call merely wraps and rethrows the exception,
> and doesn't really add any value. It's like calling fail() in a
> test-ng test.) In general an uncaught exception will cause a jtreg
> test to fail, so you might as well just declare the method with
> "throws Exception" and let them propagate. This helps reduce clutter.
> 7. Similarly I'm not sure it's necessary to check the classname
> argument for null; the failure should be apparent enough if classname
> is null. But if you think it's worth it, a concise way to null-check
> an argument is to use Objects.requireNonNull().
> 8. The "rmiComplie" method is misnamed (or typo'd); I'd suggest
> something like "runRmic".
> 9. Changing the directory of the rmic subprocess is confusing, because
> it makes different parts of the program deal with different paths to
> the same files. In addition, switching directories to the test.classes
> directory will leave the stub/tie classes there, which could possibly
> confuse future test runs, since that directory isn't necessarily
> cleared between test runs.
> The default working directory of jtreg tests is the "scratch"
> directory, which is cleared before each test run. Leaving the current
> directory as-is when running the subprocesses will write the generated
> stub/tie classes to the scratch directory, which is the right thing.
> This implies that you'll need to add the -classpath argument and a
> path to the test.classes directory to the rmic command line.
More information about the core-libs-dev