harmony-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Fedotov, Alexei A" <alexei.a.fedo...@intel.com>
Subject [Stress tests] generator review I
Date Mon, 15 May 2006 12:30:35 GMT
Alex,

Thanks for an interesting code.
>http://issues.apache.org/jira/secure/attachment/12331903/StressTestGene
rator.zip

Review of three classes follows. The issues are mostly minor with
exception to the time keeper usage - from my current vision it should be
more sophisticated.

With best regards,
Alexei Fedotov,
Intel Middleware Products Division


Test1.java:17
Package name should be org.apache.harmony.test.

ReliabilityRunner.java:25
The reason for duplication between command line parameters and
org.apache.harmony.test.ReliabilityTest.params property is given in
ReliabilityRunner.java:55. Reading this javadoc for the first time I was
confused. Probably we need just repeat that statement, or use @see
javadoc tag. BTW, javadoc was successfully generated by Eclipse.

ReliabilityRunner.java:29
The property format contains extra brackets and quotes.

ReliabilityRunner.java:29
The name of the property is inconsistent with the class name (either the
class or the property should be renamed).

ReliabilityRunner.java:41
We can remove testArgs array. Since we already reserved
org.apache.harmony.test.ReliabilityTest.params for the configuration, we
can set it using command line arguments and use it as a global storage.
In other words, the whole if statement form ReliabilityRunner.java:76
can be moved here.

ReliabilityRunner.java:59
runners.Threads is no longer in the project, probably generator.Thread
is meant

ReliabilityRunner.java:86
Reusing parametersAsString for the list of packages is confusing -
probably we need to delegate JIT optimizations like this and keep the
code understandable.

ReliabilityRunner.java:119
The problem is that deadlocked tests are counted as passed - probably we
need to signal generators that they should stop execution, and if they
fail to stop particular tests, then we need to abort VM with error
status. Aborting VM does not allow to run any more tests in this VM. The
use case is questionable but introduction of
apache.harmony.test.ReliabilityTest.params makes it possible. If we want
to consider this use case we need to try to abort a thread group first. 

Loop.java:30
produceContext, performAll - can we invent better names? produce =
perform, All adds nothing. If we are to implement signaling model, here
we should check for signaling flag instead of looping unconditionally.
Here is an example of my understanding.

    public void generate(ITest t) {
        while (ReliabilityRunner.isActive()) {
            t.test();
        }
    }

BTW, using this approach we can use IGenerator interface for generators
instead of a parent class. I like interfaces more even if they are
slower. :-) They are more flexible. 

---------------------------------------------------------------------
Terms of use : http://incubator.apache.org/harmony/mailing.html
To unsubscribe, e-mail: harmony-dev-unsubscribe@incubator.apache.org
For additional commands, e-mail: harmony-dev-help@incubator.apache.org


Mime
View raw message