[jira] Reopened: (DERBY-3211) Convert derbynet/NSinSameJVM.java to junit

["Knut Anders Hatlen (JIRA)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/DERBY-3211?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Knut Anders Hatlen reopened DERBY-3211:
---------------------------------------


Reopening the issue since there are some remaining problems:

1) Dan's comment about try/catch/println is not addressed. As the test is written now, JUnit won't ever notice if the test fails, since all exceptions are swallowed.

2) I don't think the port number and host name should be hard coded in the test. Instead of "localhost", TestConfiguration.getHostName() should be used. Also, I think we need to add a method to TestConfiguration called getAlternativePort() or something, instead of hard coding the port number 20000 in the test. The point of centralizing the allocation of port numbers in TestConfiguration, is to enable people to run multiple JUnit tests in parallel on the same machine without conflicting with each other (there was an effort some time ago to achieve this, I don't remember how far they got). If we start hard-coding port numbers in the tests, this becomes much more difficult.

I also think the new decorator methods in TestConfiguration should not take port number as an argument, as that will encourage more tests to use arbitrary port numbers. Instead we should name them differently (e.g., defaultServerDecoratorWithAlternativePort), remove the port parameter, and let them use getAlternativePort() internally.

3) Class header for NSinSameJVMTest header says DRDAProtocolTest.

4) testShutdown() has a comment which says "Just connect, do something and close the connection", but there is no code to close the connection (or the statement) as far as I can see.

5) NSinSameJVMTest.java contains a mixture of tabs/spaces, which I think should be avoided in new files.

6) The new method TestConfiguration.defaultServerDecorator(Test,int) has a comment which says "This looks bogus to me." That doesn't sound very comforting... ;)

7) Why does the test need to sleep for five seconds before it finishes? It would be good to document this in a comment.

> Convert derbynet/NSinSameJVM.java to junit
> ------------------------------------------
>
>                 Key: DERBY-3211
>                 URL: https://issues.apache.org/jira/browse/DERBY-3211
>             Project: Derby
>          Issue Type: Test
>            Reporter: Manjula Kutty
>            Assignee: Manjula Kutty
>            Priority: Minor
>             Fix For: 10.3.1.5, 10.4.0.0
>
>         Attachments: DERBY-3211_diff_11_16.txt, DERBY-3211_diff_11_19.txt, DERBY-3211_diff_11_19_ver2.txt, DERBY-3211_stat_11_16.txt, DERBY-3211_stat_11_19.txt, DERBY-3211_stat_11_19_ver2.txt
>
>


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

Re: [jira] Reopened: (DERBY-3211) Convert derbynet/NSinSameJVM.java to junit

["Dag H. Wanvik" <Da...@Sun.COM>]

"Knut Anders Hatlen (JIRA)" <ji...@apache.org> writes:

>      [ https://issues.apache.org/jira/browse/DERBY-3211?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
>
> Knut Anders Hatlen reopened DERBY-3211:
> ---------------------------------------
>
>
> Reopening the issue since there are some remaining problems:
>
> 1) Dan's comment about try/catch/println is not addressed. As the test is written now, JUnit won't ever notice if the test fails, since all exceptions are swallowed.
>
> 2) I don't think the port number and host name should be hard coded
> in the test. Instead of "localhost", TestConfiguration.getHostName()
> should be used. Also, I think we need to add a method to
> TestConfiguration called getAlternativePort() or something, instead
> of hard coding the port number 20000 in the test. The point of
> centralizing the allocation of port numbers in TestConfiguration, is
> to enable people to run multiple JUnit tests in parallel on the same
> machine without conflicting with each other (there was an effort
> some time ago to achieve this, I don't remember how far they

I did some work on this, and at that point in time, no tests used any
port except the one found in TestConfiguration. I verified this by
tracing the server code opening the port. This state of things may
have deteriorated since, though.

Thanks,
Dag