Skip to main content

Fix for Issue 36

2 replies [Last post]
Anonymous

Reply viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.
Vladimir Sizikov

Hi Dmitri,

I'm not Alexander ;), but given the high priority of this regression
and it's blocking nature (impossible to execute tests), I take a look
at it, to speed up the review cycle.

The fix indeed corrected the problem for me, and it's OK to commit.

One question for you though, I see that super.startRun is invoked from
the middle of startRun(). Typically, the super invocations are either
from the beginning or from the end of the method. Invoking it from the
middle looks fishy to me. Is there something that prevents this call
to be moved to the beginning and then the two if (isExport()) blocks
could be merged?

Thanks,
--Vladimir

On Thu, Jan 25, 2007 at 07:51:28PM +0300, Dmitri Trounine wrote:
> Alexander,
>
> please review my fix for Issue 36:
> https://cqme.dev.java.net/issues/show_bug.cgi?id=36
>
> Thanks,
> Dmitri.

> Index: src/share/classes/com/sun/tck/j2me/javatest/ExportTestSuite.java
> ===================================================================
> --- src/share/classes/com/sun/tck/j2me/javatest/ExportTestSuite.java (revision 256)
> +++ src/share/classes/com/sun/tck/j2me/javatest/ExportTestSuite.java (revision 257)
> @@ -65,13 +65,12 @@
> * Loads test export related environment entries.
> */
> protected void loadParameters(TestEnvironment env) throws Fault {
> - TestExportInfo testExportInfo = TestExportInfo.fromEnv(env);
> + testExportInfo = TestExportInfo.fromEnv(env);
> setExport(testExportInfo.isExport());
>
> super.loadParameters(env);
>
> if (testExportInfo.isExport()) {
> - this.testExportInfo = testExportInfo;
> /* Override jarSourceDir value set by
> * CldcTCKBaseTestSuite.loadParameters -- in export mode JAR files
> * will be stored in location specified by user in interview.
> @@ -113,13 +112,19 @@
> }
>
> protected void startRun(Harness harness) throws Fault {
> - File libDir = new File(testExportInfo.getExportDirName(), "lib");
> - clearDir(libDir);
> - libDir.mkdirs();
> + File libDir = null;
> +
> + if (isExport()) {
> + libDir = new File(testExportInfo.getExportDirName(), "lib");
> + clearDir(libDir);
> + libDir.mkdirs();
> + }
> super.startRun(harness);
> - String[] libJars = testExportInfo.getExportLibJars();
> - for (int i = 0; i < libJars.length; i++) {
> - copyFileToDir(new File(libJars[i]), libDir);
> + if (isExport()) {
> + String[] libJars = testExportInfo.getExportLibJars();
> + for (int i = 0; i < libJars.length; i++) {
> + copyFileToDir(new File(libJars[i]), libDir);
> + }
> }
> }
>
>

> ---------------------------------------------------------------------
> To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
> For additional commands, e-mail: meframework-help@cqme.dev.java.net

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net

Dmitri Trounine

Hi Vladimir,

Thanks for review.
It's good question about place of super.startRun()! Seems that it was
meaningfull before I made some changes in order to fit service based
test suites. Now it looks strange. I think this call to super.startRun()
can be moved to the end of the method. I'll check if this is correct and
will send a new fix for review.

Thanks,
Dmitri.

Vladimir Sizikov wrote:

>Hi Dmitri,
>
>I'm not Alexander ;), but given the high priority of this regression
>and it's blocking nature (impossible to execute tests), I take a look
>at it, to speed up the review cycle.
>
>The fix indeed corrected the problem for me, and it's OK to commit.
>
>One question for you though, I see that super.startRun is invoked from
>the middle of startRun(). Typically, the super invocations are either
>from the beginning or from the end of the method. Invoking it from the
>middle looks fishy to me. Is there something that prevents this call
>to be moved to the beginning and then the two if (isExport()) blocks
>could be merged?
>
>Thanks,
> --Vladimir
>
>On Thu, Jan 25, 2007 at 07:51:28PM +0300, Dmitri Trounine wrote:
>
>
>>Alexander,
>>
>>please review my fix for Issue 36:
>>https://cqme.dev.java.net/issues/show_bug.cgi?id=36
>>
>>Thanks,
>> Dmitri.
>>
>>
>
>
>
>>Index: src/share/classes/com/sun/tck/j2me/javatest/ExportTestSuite.java
>>===================================================================
>>--- src/share/classes/com/sun/tck/j2me/javatest/ExportTestSuite.java (revision 256)
>>+++ src/share/classes/com/sun/tck/j2me/javatest/ExportTestSuite.java (revision 257)
>>@@ -65,13 +65,12 @@
>> * Loads test export related environment entries.
>> */
>> protected void loadParameters(TestEnvironment env) throws Fault {
>>- TestExportInfo testExportInfo = TestExportInfo.fromEnv(env);
>>+ testExportInfo = TestExportInfo.fromEnv(env);
>> setExport(testExportInfo.isExport());
>>
>> super.loadParameters(env);
>>
>> if (testExportInfo.isExport()) {
>>- this.testExportInfo = testExportInfo;
>> /* Override jarSourceDir value set by
>> * CldcTCKBaseTestSuite.loadParameters -- in export mode JAR files
>> * will be stored in location specified by user in interview.
>>@@ -113,13 +112,19 @@
>> }
>>
>> protected void startRun(Harness harness) throws Fault {
>>- File libDir = new File(testExportInfo.getExportDirName(), "lib");
>>- clearDir(libDir);
>>- libDir.mkdirs();
>>+ File libDir = null;
>>+
>>+ if (isExport()) {
>>+ libDir = new File(testExportInfo.getExportDirName(), "lib");
>>+ clearDir(libDir);
>>+ libDir.mkdirs();
>>+ }
>> super.startRun(harness);
>>- String[] libJars = testExportInfo.getExportLibJars();
>>- for (int i = 0; i < libJars.length; i++) {
>>- copyFileToDir(new File(libJars[i]), libDir);
>>+ if (isExport()) {
>>+ String[] libJars = testExportInfo.getExportLibJars();
>>+ for (int i = 0; i < libJars.length; i++) {
>>+ copyFileToDir(new File(libJars[i]), libDir);
>>+ }
>> }
>> }
>>
>>
>>
>>
>
>
>
>>---------------------------------------------------------------------
>>To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
>>For additional commands, e-mail: meframework-help@cqme.dev.java.net
>>
>>
>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
>For additional commands, e-mail: meframework-help@cqme.dev.java.net
>
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: meframework-unsubscribe@cqme.dev.java.net
For additional commands, e-mail: meframework-help@cqme.dev.java.net