Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: neutral (athena.apache.org: local policy)
Message-ID: <5016EBB0.80008@apache.org>
Date: Mon, 30 Jul 2012 21:16:48 +0100
From: Mark Thomas <markt@apache.org>
User-Agent: Mozilla/5.0 (Windows NT 5.2; WOW64;
 rv:14.0) Gecko/20120713 Thunderbird/14.0
MIME-Version: 1.0
To: Commons Developers List <dev@commons.apache.org>
Subject: Re: [pool] Question / potential improvement on KeyedObjectPool
References: <CC3C33B7.17F19%liviu.tudor@gmail.com>
In-Reply-To: <CC3C33B7.17F19%liviu.tudor@gmail.com>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

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. 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-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org