Re: [P-unit-devel] SF.net SVN: p-unit: [318] trunk/punit/src/org/punit/method/runner

[Chris W. <ch...@cj...>]

Hi,

I am writing tests that need to run both a unit tests and performance
tests, so we are writing them so that the operation includes various
bits of setup in the setUp method.  I can see that the instance below
makes sense to just run the setup once, but likewise your example
won't work if run as a unit test with junit.

I think just measuring the time to execute the test method would be
ideal, but depending on the scenario, as long as you know that every
run is doing the same thing you can extrapolate how long it takes.
You could, for instance, have an empty test and then the time for that
would give you a baseline to compaire other tests against (as it will
be doing all the setUp etc and method and thread initialisation
stuff).

I would say that if there needs to be a setup method only run once for
all performace tests either the beforeClass method or a special method
should be used.  I would just expect a setUp or @Before method to run
before all tests.

If I were to have a class with two tests in it testA and testB, what
would the run order:

setUp
testA
testA
testA
tearDown

setUp
testB
testB
testB
tearDown

or something else?

I guess I could live with this, actually, though it doesn't feel quite
right, as long as the setUp method is run in the same thread as the
test method.

Chris

2008/6/11 Andrew Zhang <zha...@gm...>:
>
>
> On Wed, Jun 11, 2008 at 10:25 PM, Chris Wilson <ch...@cj...> wrote:
>>
>> Hi,
>>
>> I agree that the setUp/tearDown should not be timed - I hadn't though
>> of that in my code.  I wasn't massively clear as to what a lot of the
>> watcher etc code was doing.  It does, however, make a lot of sense to
>> have the setUp/tearDown run once per test run - ie if you run testA
>> method five times you should run the setUp and TearDown five times
>> too.  That would be the expected behaviour, I would think, as that is
>> what happens when the tests are run in general.
>>
>>
>> Perhaps what needs to happen is that all the watcher code needs to be
>> run in the same thread as the test?
>
> Hi Chris,
>
> My intuition is that multiple setUp/tearDown makes sense for some cases, but
> single run also is reasonable sometimes. :)
>
> Take testing ArrayList#add method as an example:
> setUp: new an ArrayList instance
> testAdd: list.add(someObject); // run 10 times
> checkAdd: assert the list size is 10
>
> The test makes sure that concurrent invocation on list.add doesn't throw any
> exception, and the result list size is 10.
> For this example, if ArrayList is replaced with Vector, it may very possible
> fail because Vector#add is not synchronized and may throw some exception
> during add (I can't remember the exact exception).
>
> Would you please explain your scenario a little bit? Thanks!
>
> Another point is that which kind of time do we want to measure?
>
> setUp -> testAdd 10 times-> checkAdd -> tearDown
>
> The original code measures the time/memory to execute testAdd 10 times.
>
> What's your idea? Thanks a lot!
>
> p.s. sorry, I forget to add p-unit-dev in the receiver list of my last mail.
> Anyway, you have quoted it. :)
>>
>>
>> Chris
>>
>> 2008/6/11 Andrew Zhang <zha...@gm...>:
>> >
>> >
>> > On Wed, Jun 11, 2008 at 4:07 PM, Chris Wilson <ch...@cj...>
>> > wrote:
>> >>
>> >> Hi,
>> >>
>> >> No, I removed the reverence to setup in the setUpBeforeWatchers method
>> >> (and tearDown in the setUpAfterWatchers too).  The logic part of it is
>> >> done by the doNormalSetupAndTeardown method and then based on that
>> >> it'll run them or not.
>> >
>> > Hi Chris,
>> >
>> > SetUp/tearDown method is usually considered as preparation/cleanup of a
>> > test
>> > case so that p-unit only measures the execution time and memory of test
>> > method instead of test method + setUp/tearDown.
>> >
>> > setUpBeforeWatchers(params);
>> > startWatchers(testInstance, method, params); // begin to measure
>> > setUpAfterWatchers(params);
>> > startToStopThread();
>> > runImpl();
>> > stopToStopThread();
>> >
>> > Another point you addressed is that should concurrent test method run
>> > setUp/tearDown multiple times or single time?
>> > I'm not 100% sure when I designed ConcurrentRunner. For your case, it
>> > looks
>> > like that you prefer multiple option. Do I understand correctly? Thanks!
>> >
>> >
>> >
>> >>
>> >>
>> >> Chris
>> >>
>> >> 2008/6/11 Andrew Zhang <zha...@gm...>:
>> >> > Hi Chris,
>> >> >
>> >> > In original code of AbstractMethodRunner#run, it invokes setup as
>> >> > well:
>> >> >
>> >> >                 init(testInstance, method, params);
>> >> >                 setUpBeforeWatchers(params); // setup will be run
>> >> > here.
>> >> >                 startWatchers(testInstance, method, params);
>> >> >                 setUpAfterWatchers(params);
>> >> >                 startToStopThread();
>> >> >                 runImpl();
>> >> >                 stopToStopThread();
>> >> >                 runCheckMethod(testInstance, params);
>> >> >
>> >> > Will your modification cause multiple run of setUp & tearDown?
>> >> > Thanks!
>> >> >
>> >> > On Tue, Jun 10, 2008 at 4:22 PM, <cu...@us...>
>> >> > wrote:
>> >> >>
>> >> >> Revision: 318
>> >> >>          http://p-unit.svn.sourceforge.net/p-unit/?rev=318&view=rev
>> >> >> Author:   cuvavu
>> >> >> Date:     2008-06-10 01:22:58 -0700 (Tue, 10 Jun 2008)
>> >> >>
>> >> >> Log Message:
>> >> >> -----------
>> >> >> Making the concurrentRunner run before and after stuff in the same
>> >> >> thread
>> >> >> as the actual test.
>> >> >>
>> >> >> Modified Paths:
>> >> >> --------------
>> >> >>    trunk/punit/src/org/punit/method/runner/AbstractMethodRunner.java
>> >> >>
>> >> >>  trunk/punit/src/org/punit/method/runner/ConcurrentMethodRunner.java
>> >> >>    trunk/punit/src/org/punit/method/runner/SoloMethodRunner.java
>> >> >>
>> >> >> Modified:
>> >> >> trunk/punit/src/org/punit/method/runner/AbstractMethodRunner.java
>> >> >> ===================================================================
>> >> >> ---
>> >> >> trunk/punit/src/org/punit/method/runner/AbstractMethodRunner.java
>> >> >> 2008-06-10 08:22:52 UTC (rev 317)
>> >> >> +++
>> >> >> trunk/punit/src/org/punit/method/runner/AbstractMethodRunner.java
>> >> >> 2008-06-10 08:22:58 UTC (rev 318)
>> >> >> @@ -46,7 +46,7 @@
>> >> >>
>> >> >>     protected transient Class<? extends Throwable>
>> >> >> _expectedException;
>> >> >>
>> >> >> -    protected abstract void runImpl() throws Throwable;
>> >> >> +    protected abstract void runImpl( boolean
>> >> >> doNormalSetupAndTeardown
>> >> >> )
>> >> >> throws Throwable;
>> >> >>
>> >> >>     public AbstractMethodRunner() {
>> >> >>         _watchers.add(_timeWatcher);
>> >> >> @@ -79,7 +79,7 @@
>> >> >>                 startWatchers(testInstance, method, params);
>> >> >>                 setUpAfterWatchers(params);
>> >> >>                 startToStopThread();
>> >> >> -                runImpl();
>> >> >> +                runImpl( doNormalSetupAndTeardown(params) );
>> >> >>                 stopToStopThread();
>> >> >>                 runCheckMethod(testInstance, params);
>> >> >>             } else {
>> >> >> @@ -171,10 +171,15 @@
>> >> >>         } else if (_convention.isParameterizedTest(_class)) {
>> >> >>             ((Parameterized) _testInstance)
>> >> >>                     .setUpBeforeWatchers((Parameter) params[0]);
>> >> >> -        } else {
>> >> >> -            setUp();
>> >> >>         }
>> >> >>     }
>> >> >> +
>> >> >> +    private boolean doNormalSetupAndTeardown(Object[] params)
>> >> >> throws
>> >> >> Throwable {
>> >> >> +        if (_convention.isPUnitTest(_class) ||
>> >> >> _convention.isParameterizedTest(_class)) {
>> >> >> +            return false;
>> >> >> +        }
>> >> >> +        return true;
>> >> >> +    }
>> >> >>
>> >> >>     private void setUpAfterWatchers(Object[] params) throws
>> >> >> Throwable {
>> >> >>         if (_convention.isPUnitTest(_class)) {
>> >> >> @@ -200,12 +205,10 @@
>> >> >>         } else if (_convention.isParameterizedTest(_class)) {
>> >> >>             ((Parameterized) _testInstance)
>> >> >>                     .tearDownAfterWatchers((Parameter) params[0]);
>> >> >> -        } else {
>> >> >> -            tearDown();
>> >> >>         }
>> >> >>     }
>> >> >>
>> >> >> -    private void setUp() throws Throwable {
>> >> >> +    protected void setUp() throws Throwable {
>> >> >>         if (_setUpMethods != null) {
>> >> >>                for (Method setUpMethod : _setUpMethods) {
>> >> >>                    try {
>> >> >> @@ -218,7 +221,7 @@
>> >> >>         }
>> >> >>     }
>> >> >>
>> >> >> -    private void tearDown() throws Throwable {
>> >> >> +    protected void tearDown() throws Throwable {
>> >> >>         if (_tearDownMethods != null) {
>> >> >>                for (Method tearDownMethod : _tearDownMethods) {
>> >> >>                    try {
>> >> >>
>> >> >> Modified:
>> >> >> trunk/punit/src/org/punit/method/runner/ConcurrentMethodRunner.java
>> >> >> ===================================================================
>> >> >> ---
>> >> >> trunk/punit/src/org/punit/method/runner/ConcurrentMethodRunner.java
>> >> >> 2008-06-10 08:22:52 UTC (rev 317)
>> >> >> +++
>> >> >> trunk/punit/src/org/punit/method/runner/ConcurrentMethodRunner.java
>> >> >> 2008-06-10 08:22:58 UTC (rev 318)
>> >> >> @@ -22,21 +22,21 @@
>> >> >>         _concurrentCount = concurrentCount;
>> >> >>     }
>> >> >>
>> >> >> -       protected void runImpl() throws Throwable {
>> >> >> -        startThreads();
>> >> >> +       protected void runImpl( boolean doNormalSetupAndTeardown )
>> >> >> throws
>> >> >> Throwable {
>> >> >> +        startThreads( doNormalSetupAndTeardown );
>> >> >>         joinThreads();
>> >> >>         if (_concurrentException.size() > 0) {
>> >> >>             throw _concurrentException;
>> >> >>         }
>> >> >>     }
>> >> >>
>> >> >> -    private void startThreads() {
>> >> >> +    private void startThreads( boolean doNormalSetupAndTeardown ) {
>> >> >>         int count = _convention.getConcurrentCount(_testInstance,
>> >> >> _method);
>> >> >>         int threadCount = count > 0 ? count : _concurrentCount;
>> >> >>         _threads = new TestMethodThread[threadCount];
>> >> >>         _barrier = new CyclicBarrier(threadCount);
>> >> >>         for (int i = 0; i < _threads.length; ++i) {
>> >> >> -            _threads[i] = new TestMethodThread("punit concurrent
>> >> >> method
>> >> >> runner"); //$NON-NLS-1$
>> >> >> +            _threads[i] = new TestMethodThread("punit concurrent
>> >> >> method
>> >> >> runner", doNormalSetupAndTeardown); //$NON-NLS-1$
>> >> >>         }
>> >> >>         for (int i = 0; i < _threads.length; ++i) {
>> >> >>             _threads[i].start();
>> >> >> @@ -59,14 +59,26 @@
>> >> >>
>> >> >>     private class TestMethodThread extends Thread {
>> >> >>
>> >> >> -        public TestMethodThread(String threadName) {
>> >> >> +       boolean doNormalSetupAndTeardown;
>> >> >> +
>> >> >> +        public TestMethodThread(String threadName, boolean
>> >> >> doNormalSetupAndTeardown) {
>> >> >>             super(threadName);
>> >> >> +            this.doNormalSetupAndTeardown =
>> >> >> doNormalSetupAndTeardown;
>> >> >>         }
>> >> >>
>> >> >>         public void run() {
>> >> >>             try {
>> >> >>                _barrier.await();
>> >> >> -                ConcurrentMethodRunner.this.runMethod();
>> >> >> +               try {
>> >> >> +                               if (doNormalSetupAndTeardown) {
>> >> >> +                                       setUp();
>> >> >> +                               }
>> >> >> +
>> >> >> ConcurrentMethodRunner.this.runMethod();
>> >> >> +                       } finally {
>> >> >> +                               if (doNormalSetupAndTeardown) {
>> >> >> +                                       tearDown();
>> >> >> +                               }
>> >> >> +                       }
>> >> >>             } catch (Throwable t) {
>> >> >>                 _concurrentException.add(t);
>> >> >>             }
>> >> >>
>> >> >> Modified:
>> >> >> trunk/punit/src/org/punit/method/runner/SoloMethodRunner.java
>> >> >> ===================================================================
>> >> >> --- trunk/punit/src/org/punit/method/runner/SoloMethodRunner.java
>> >> >> 2008-06-10 08:22:52 UTC (rev 317)
>> >> >> +++ trunk/punit/src/org/punit/method/runner/SoloMethodRunner.java
>> >> >> 2008-06-10 08:22:58 UTC (rev 318)
>> >> >> @@ -8,7 +8,16 @@
>> >> >>
>> >> >>        private static final long serialVersionUID =
>> >> >> 3278612571978181393L;
>> >> >>
>> >> >> -       protected void runImpl() throws Throwable {
>> >> >> -               runMethod();
>> >> >> +       protected void runImpl( boolean doNormalSetupAndTeardown )
>> >> >> throws
>> >> >> Throwable {
>> >> >> +       try {
>> >> >> +                       if (doNormalSetupAndTeardown) {
>> >> >> +                               setUp();
>> >> >> +                       }
>> >> >> +                       runMethod();
>> >> >> +               } finally {
>> >> >> +                       if (doNormalSetupAndTeardown) {
>> >> >> +                               tearDown();
>> >> >> +                       }
>> >> >> +               }
>> >> >>        }
>> >> >>  }
>> >> >>
>> >> >>
>> >> >> This was sent by the SourceForge.net collaborative development
>> >> >> platform,
>> >> >> the world's largest Open Source development site.
>> >> >>
>> >> >>
>> >> >>
>> >> >> -------------------------------------------------------------------------
>> >> >> Check out the new SourceForge.net Marketplace.
>> >> >> It's the best place to buy or sell services for
>> >> >> just about anything Open Source.
>> >> >> http://sourceforge.net/services/buy/index.php
>> >> >> _______________________________________________
>> >> >> P-unit-devel mailing list
>> >> >> P-u...@li...
>> >> >> https://lists.sourceforge.net/lists/listinfo/p-unit-devel
>> >> >
>> >> >
>> >> >
>> >> > --
>> >> > Best regards,
>> >> > Andrew Zhang
>> >> >
>> >> > db4o - database for Android: www.db4o.com
>> >> > http://zhanghuangzhu.blogspot.com/
>> >
>> >
>> >
>> > --
>> > Best regards,
>> > Andrew Zhang
>> >
>> > db4o - database for Android: www.db4o.com
>> > http://zhanghuangzhu.blogspot.com/
>
>
>
> --
> Best regards,
> Andrew Zhang
>
> db4o - database for Android: www.db4o.com
> http://zhanghuangzhu.blogspot.com/