On 30 July 2012 21:16, Mark Thomas <ma...@apache.org> wrote: > On 30/07/2012 21:07, Liviu Tudor wrote: >> * Findbugs: indeed, some of the findbugs warnings are not entirely correct >> -- and we should probably look at using the Findbugs annotations to >> manually annotate the ones which we know to actually be false alarms? >> There were also a couple related to performance suggestions which were >> valid -- so as per your suggestion I'm going to submit a patch for those. >> Regarding the false alerts in FindBugs -- there are 2 ways to proceed with >> "removing" these alerts: >> ** one is as I suggested above to enable the findbugs annotations in the >> pom and manually annotate those 2 methods to remove the findbugs checks >> ** second one is to manually remove the findbugs checks via a >> findbugs-exclude.xml from the suite of findbugs tests run. >> Not sure which one do we prefer in Commons? (or more to the point in this >> case?) > > Personally, I prefer findbugs-exclude.xml since that doesn't pollute the > build for folks who have no interest in FindBugs. > >> * The unit test thing though puzzles me as I keep constantly get this : >> >> Results : >> >> Failed tests: >> testConstructors(org.apache.commons.pool.impl.TestGenericKeyedObjectPool): >> expected:<4> but was:<2> >> >> testConstructors(org.apache.commons.pool.impl.TestGenericKeyedObjectPoolFac >> tory): expected:<4> but was:<2> >> >> testConstructors(org.apache.commons.pool.impl.TestGenericObjectPoolFactory) >> : expected:<4> but was:<2> >> >> >> When running the TestGenericKeyedObjectPool! I'm running them on a Mac -- >> but checked them on a win machine with the same result. Looking closer, it >> seems to be down to the fact that the maxIdle is set to be lower than >> midIdle -- and there is logic in the GenericKeyedObjectPool to return >> maxIdle in such cases. I'm not sure therefore if the default values in the >> unit test are wrong or the logic in the GenericKeyedObjectPool is wrong? >> I'm happy to change the assert's in the unit test and submit a patch -- >> but I would like this validated by someone who has been involved >> previously in this piece of code? >> Also, I would be curious to know whether the tests are indeed passing ? > > They fail for me to. Odd. I would have expected a CI nag about that. > > 'svn log' and 'svn blame' can be very useful when determining why code > is the way it is. In this case they point to POOL-212 and it is indeed > the unit tests that are wrong. > >> * Last but not the least, I see the pom references junit-4.10 but the unit >> tests are still based on the old junit-3.8 looking at it. Worth to change >> these to annotations a la junit-4? > > Not for Pool 1.5. The target Java version is < 1.5.
In which case, it should not be using JUnit 4.10, which does not run on Java 1.4; the most recent JUnit for Java 1.4 is 3.8.2. > Not sure it is worth > it for 1.6 (and it easier to keep the 1.5.x and 1.6.x in sync if it is > just generics that is the difference). For Pool 2.x I don't have a > strong preference. > > Mark > > > --------------------------------------------------------------------- > 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
