On 13.07.10 22:33, Tiago Espinha wrote:
I think ideally we'd keep the max number of ports on a tight fit to what is
actually needed, that's why I left them at 10. This way if new ports are
required along the way, whoever makes the changes gets alerted that they need to
increase this constant.

Of course we can also overshoot and give it a large margin and possibly never
get alerted by it again but I tend to prefer the tight fit approach. If we are
introducing tests that require more ports, it's a good idea to alert the
developer that it might have some repercussions in terms of parallel runs and of
how far apart the port ranges should be.

I agree with this reasoning. After all, only a few tests are actually requesting additional ports.

It's interesting though, even if you define a base port through a property, this
issue should still come up because you'll still be using more ports than the max
allows...

That's true if you run all the suites in suites.All in serial, but when you run tests in parallel each suite gets it's own TestConfiguration (with its own base port). Depending on which tests require an additional port (calling getNextAvailablePort(), directly or indirectly), a parallel run may or may not trigger the assert.

Returning to keeping the number of ports down, would it make sense to store the bogus port in a static variable? This way, all the TestConfiguration instances in a JVM would share the same port, which is possible since it is reserved as a port where nothing is supposed to be listening.


Regards,
--
Kristian

Tiago


----- Original Message ----
From: Kristian Waagan<kristian.waa...@oracle.com>
To: derby-dev@db.apache.org
Sent: Tue, 13 July, 2010 20:25:47
Subject: Re: regression test regressed

On 13.07.10 20:59, Tiago Espinha wrote:
Hello Rick,

Do we know which patch it was that introduced this? The second failure has to
do
with my test conversions from last year and it should come up if we're trying
to
request one more port from TestConfiguration.getNextAvailablePort() without
adjusting the constant for the maximum number of ports needed.

This was surely introduced by my commit for DERBY-4700.

This is as intended, to make sure all the ports used are sequential to allow
for
parallel test runs.

If this is just the result of a new test invoking the getNextAvailablePort()
then all there needs to be done is increment the
TestConfiguration.MAX_PORTS_USED constant to 11 and the following wiki page
also
should be updated for
coherence: http://wiki.apache.org/db-derby/DerbyJUnitTesting#Running_Tests

I guess we just have to bump the number of ports we are allowed to use.
The patched caused two more ports to be allocated (I think, it's not
quite clear to me how many TestConfiguration instances are created).
Shall we increase it to 12, 15 or 20?

Now, the reason why it didn't fail when I ran the test, is of course
that I used the new parallel run capability. In that case, each suite
gets 10 ports for it self.


Sorry for the noise, I'll fix this tomorrow.
If someone wants to quickly bump the constant, feel free to do so under
DERBY-4700 (and update the wiki as Tiago mentioned). As a last step, the
example script provided by Knut for parallel runs should also be updated.



Reply via email to