I'm looking at unit test these days with a broader POV. I like to see code like this as examples (that need to be documented as you point out.) These stay valid unlike text because they must compile even if they are @Ignore'd in this particular case. So I'd prefer to see them stay but I'll go with the consensus if that is what this thread decides.
Gary On Nov 13, 2011, at 11:09, Phil Steitz <phil.ste...@gmail.com> wrote: > On 11/12/11 8:05 PM, Gary Gregory wrote: >> Hi Jorge, >> >> The patch I see here: >> >> http://www.mail-archive.com/dev@commons.apache.org/msg26521.html >> >> does not apply due to formatting in the email (line lengths I think). >> >> Can you send me (ggregory at apache dor org) the patch (based on HEAD of >> trunk) or post a file somewhere? > > My HO is that this test should stay with the ticket and not be added > to [lang]'s test suite. It illustrates just one kind of problem > that can happen. Its easy enough for people working on a general > fix (which I personally think is impossible) to apply the patch > locally. Alternatively, we could just disable the test altogether > in the test sources. If we do decide to leave it in the test > sources, we should probably either add a comment indicating that it > only illustrates one kind of problem or change the internal > synchronization of the test class to use explicit locking via > private Lock instances, since as is it sort of makes it look like > just synchronizing the [lang] code on the instance's monitor could > solve the problem. > > Phil >> >> Thank you, >> Gary >> >> On Sat, Nov 12, 2011 at 7:07 AM, Jörg Schaible<joerg.schai...@gmx.de>wrote: >> >>> Ping, Gary? >>> >>> Jörg Schaible wrote: >>> >>>> Hi Gary, >>>> >>>> Gary Gregory wrote: >>>> >>>>> For now, I've @Ignore'd >>>>> ReflectionToStringBuilderConcurrencyTest.testCopyOnWriteArrayList() >>>>> >>>>> FYI: If I reduce the DATA_SIZE from 100000 to 10000 the test >>>>> ReflectionToStringBuilderConcurrencyTest.testLinkedList() does not >>> always >>>>> fail for me. >>>> For me it seems to do (I modified the other tests to drop the @Ignore) if >>>> I take an object that has to do a bit more in toString: >>>> >>>> ============ %< ============>>> $ svn diff >>>> Index: >>>> >>> src/test/java/org/apache/commons/lang3/builder/ReflectionToStringBuilderConcurrencyTest.java >>>> ==================================================================>>> --- >>>> >>> src/test/java/org/apache/commons/lang3/builder/ReflectionToStringBuilderConcurrencyTest.java >>>> (revision 1200195) >>>> +++ >>>> >>> src/test/java/org/apache/commons/lang3/builder/ReflectionToStringBuilderConcurrencyTest.java >>>> (working copy) >>>> @@ -18,7 +18,9 @@ >>>> package org.apache.commons.lang3.builder; >>>> >>>> import java.util.ArrayList; >>>> +import java.util.Calendar; >>>> import java.util.Collection; >>>> +import java.util.ConcurrentModificationException; >>>> import java.util.LinkedList; >>>> import java.util.List; >>>> import java.util.concurrent.Callable; >>>> @@ -30,7 +32,6 @@ >>>> >>>> import junit.framework.Assert; >>>> >>>> -import org.junit.Ignore; >>>> import org.junit.Test; >>>> >>>> /** >>>> @@ -58,33 +59,48 @@ >>>> } >>>> } >>>> >>>> - private static final int DATA_SIZE = 100000; >>>> + private static final int DATA_SIZE = 10000; >>>> private static final int REPEAT = 100; >>>> >>>> @Test >>>> - @Ignore >>>> - public void testLinkedList() throws InterruptedException, >>>> ExecutionException { >>>> - this.testConcurrency(new CollectionHolder<List<Integer>>(new >>>> LinkedList<Integer>())); >>>> + public void testLinkedList() throws InterruptedException { >>>> + try { >>>> + this.testConcurrency(new >>> CollectionHolder<List<Calendar>>(new >>>> LinkedList<Calendar>())); >>>> + Assert.fail("Thrown " + ExecutionException.class.getName() + >>>> " expected"); >>>> + } catch (final ExecutionException e) { >>>> + Assert.assertTrue( >>>> + ConcurrentModificationException.class.getName() + " >>>> expected as cause", >>>> + e.getCause() instanceof >>> ConcurrentModificationException); >>>> + } >>>> } >>>> >>>> @Test >>>> - @Ignore >>>> - public void testArrayList() throws InterruptedException, >>>> ExecutionException { >>>> - this.testConcurrency(new CollectionHolder<List<Integer>>(new >>>> ArrayList<Integer>())); >>>> + public void testArrayList() throws InterruptedException { >>>> + try { >>>> + this.testConcurrency(new >>>> CollectionHolder<List<Calendar>>(new ArrayList<Calendar>())); >>>> + Assert.fail("Thrown " + ExecutionException.class.getName() + >>>> " expected"); >>>> + } catch (final ExecutionException e) { >>>> + Assert.assertTrue( >>>> + ConcurrentModificationException.class.getName() + " >>>> expected as cause", >>>> + e.getCause() instanceof >>> ConcurrentModificationException); >>>> + } >>>> } >>>> >>>> @Test >>>> - @Ignore >>>> public void testCopyOnWriteArrayList() throws InterruptedException, >>>> ExecutionException { >>>> - this.testConcurrency(new CollectionHolder<List<Integer>>(new >>>> CopyOnWriteArrayList<Integer>())); >>>> + this.testConcurrency(new CollectionHolder<List<Calendar>>(new >>>> CopyOnWriteArrayList<Calendar>())); >>>> } >>>> >>>> - private void testConcurrency(final CollectionHolder<List<Integer>> >>>> holder) throws InterruptedException, >>>> + private void testConcurrency(final CollectionHolder<List<Calendar>> >>>> holder) throws InterruptedException, >>>> ExecutionException { >>>> - final List<Integer> list = holder.collection; >>>> + final List<Calendar> list = holder.collection; >>>> + final Calendar cal = Calendar.getInstance(); >>>> + cal.clear(Calendar.MILLISECOND); >>>> + cal.set(2000, Calendar.JANUARY, 1, 0, 0, 0); >>>> // make a big array that takes a long time to toString() >>>> for (int i = 0; i< DATA_SIZE; i++) { >>>> - list.add(Integer.valueOf(i)); >>>> + list.add((Calendar)cal.clone()); >>>> + cal.add(Calendar.HOUR_OF_DAY, 1); >>>> } >>>> // Create a thread pool with two threads to cause the most >>>> contention on the underlying resource. >>>> final ExecutorService threadPool >>> >>>> Executors.newFixedThreadPool(2); >>>> ============ %< ============>>> >>>> I'll have to retry on a faster machine though. Drills down CopyOnWrite >>>> test to ~1s. >>>> >>>> Cheers, >>>> Jörg >>> >>> >>> --------------------------------------------------------------------- >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org >>> For additional commands, e-mail: dev-h...@commons.apache.org >>> >>> >> > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org > For additional commands, e-mail: dev-h...@commons.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
