Re: Code coverage for client.net package

[Mohamed Nufail]

Hi,


On Fri, Jul 6, 2012 at 12:45 PM, Knut Anders Hatlen
wrote:

>
> Instead of copying code from PrepareStatementTest, I'm wondering if we
> could rather reuse the code directly by tweaking PrepareStatementTest's
> suite() method to run the test cases with a decorator that makes them
> use pooled connections. Something along the lines of:
>
> suite.addTest(TestConfiguration.clientServerDecorator(
> TestConfiguration.connectionCPDecorator(new CleanDatabaseTestSetup(
> new TestSuite(PrepareStatementTest.class);
>
>
> Thank you. That seems like a better way of doing it. I tested it with
PrepareStatementTest and LogicalPreparedStatement does get covered.

I also tried it with jdbc4.PreparedStatementTest and
jdbc4.CallableStatementTest. Again everything is fine with coverage of
LogicalPreparedStatement. But LogicalPreparedStatement40 and
LogicalCallableStatement classes doesn't seem to get properly covered. Only
few methods in those classes are shown as covered in emma. But for some
methods shown as not covered, the corresponding methods in the physical
statement classes are showed as covered.
I think when pooled connection is used it should create logical statements
and under JDBC4 it should create 40 statements. So all calls should go
through those classes. Is there a reason for coverage report showing no
coverage for some methods, which are called in the test classes and whose
physical statement methods are also covered?


>
> If they could be created with some of the existing decorators, I think
> that would be preferable. The advantage of using the decorators is that
> the new tests won't be limited to running on LogicalStatements and could
> test all our classes that implement java.sql.Statement, which gives us
> some additional testing. Also, if we use the same test code for all
> kinds of statements, we can detect if the different statement
> implementations behave differently, which we try to avoid.
>
> --
> Knut Anders
>



-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Knut Anders Hatlen]

Mohamed Nufail  writes:

> Hi,
>
> I moved onto implementing some tests for the client.am package. I
> first tried to cover the LogicalPreparedStatement class. I'm still
> working on it and uploaded the current version on the following link.
> http://pastebin.com/w4JqSh5Z
>
> In writing the above test, I looked into the following tests which are
> similar and tried to follow them.
> - org.apache.derby.client.am.LogicalStatementEntityTest
> -
> org.apache.derbyTesting.functionTests.tests.derbynet.PrepareStatementTest
> -
> org.apache.derbyTesting.functionTests.tests.jdbc4.PreparedStatementTest
>
> Most of the testParameterTypes() method is taken from
> PrepareStatementTest#testParameterTypes().

Instead of copying code from PrepareStatementTest, I'm wondering if we
could rather reuse the code directly by tweaking PrepareStatementTest's
suite() method to run the test cases with a decorator that makes them
use pooled connections. Something along the lines of: 

suite.addTest(TestConfiguration.clientServerDecorator(
TestConfiguration.connectionCPDecorator(new CleanDatabaseTestSetup(
new TestSuite(PrepareStatementTest.class);

> testUnsupportedParameterTypes() is supposed to invoke JDBC4 specific
> methods in LogicalPreparedStatement40. So it should be run only in JVM
> which supports it. How do I enforce this?

There's a helper method JDBC.vmSupportsJDBC4() that can be used to check
if JDBC 4 is supported on the JVM. You could either check it in the
suite() method and only add the test case if it's expected to succeed,
or you could check it at the start of the test case and return early if
it needs to be skipped.

Sometimes, though, the JDBC 4 test cases must be moved to a separate
class, since the test class may not even load on older JVMs if the test
uses classes/methods that don't exist on the older JVM.

> Moreover, to create a LogicalPreparedStatement, I've now used
> ClientPooledConnection and CachingLogicalConnection in the method
> createLogicalPreparedStatement(). But in LogicalStatementEntityTest,
> this is done straightly through StatementCacheInteractor. But those
> methods are package-private. So I had to use this methodology.

If they could be created with some of the existing decorators, I think
that would be preferable. The advantage of using the decorators is that
the new tests won't be limited to running on LogicalStatements and could
test all our classes that implement java.sql.Statement, which gives us
some additional testing. Also, if we use the same test code for all
kinds of statements, we can detect if the different statement
implementations behave differently, which we try to avoid.

> I would be including couple more tests to this and do some clean-up
> and hope to write a similar test to cover LogicalCallableStatement
> also. But I would like to get feedback on my current progress. I'd
> like to know if I'm going down the correct path.
>
> Regards,
> Nufail.

-- 
Knut Anders

Re: Code coverage for client.net package

[Mohamed Nufail]

Hi,

I moved onto implementing some tests for the client.am package. I first
tried to cover the LogicalPreparedStatement class. I'm still working on it
and uploaded the current version on the following link.
http://pastebin.com/w4JqSh5Z

In writing the above test, I looked into the following tests which are
similar and tried to follow them.
- org.apache.derby.client.am.LogicalStatementEntityTest
- org.apache.derbyTesting.functionTests.tests.derbynet.PrepareStatementTest
- org.apache.derbyTesting.functionTests.tests.jdbc4.PreparedStatementTest

Most of the testParameterTypes() method is taken from
PrepareStatementTest#testParameterTypes().

testUnsupportedParameterTypes() is supposed to invoke JDBC4 specific
methods in LogicalPreparedStatement40. So it should be run only in JVM
which supports it. How do I enforce this?

Moreover, to create a LogicalPreparedStatement, I've now used
ClientPooledConnection and CachingLogicalConnection in the method
createLogicalPreparedStatement(). But in LogicalStatementEntityTest, this
is done straightly through StatementCacheInteractor. But those methods are
package-private. So I had to use this methodology.

I would be including couple more tests to this and do some clean-up and
hope to write a similar test to cover LogicalCallableStatement also. But I
would like to get feedback on my current progress. I'd like to know if I'm
going down the correct path.

Regards,
Nufail.

-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Bryan Pendleton]

On 06/18/2012 11:22 AM, Katherine Marsden wrote:

I believe with ProtocolTest the client is not invoked at all but rather 
replaces the client, so it can only be used to help
cover server side code.


Oh! Good catch, Kathey! Thanks for saving us from heading down a dead end.


> I suppose some similar test framework.could be written to throw unexpected 
protocol at the client to exercise some of the client
> code to detect a misbehaved server but I don't think it is worth it. In 
addition this particular case is geared to detecting the
> client gives a proper response to to the server's proper response to the 
client misbehaving, so I think it is just one of those
> cases that ok to leave uncovered.
>
> I would prefer to see you focus on code paths for positive cases than 
extremely remote negative protocol errors.
>
> So if looking at NetConnectionReply, I would say focus first on methods that 
are not covered at all and do not sound protocol
> error related for example, in that file I see parseRDBATHRM which I would 
think we would cover for an authentication failure and
> something called verifyConnectReply(). Is that called anywhere and what 
should it do? You might want to look at a higher level
> am.Connection and net.Connection. What's not covered there?

Excellent ideas.

+1, and thanks for the suggestions!

bryan

Re: Code coverage for client.net package

[Katherine Marsden]

On 6/17/2012 10:07 AM, Bryan Pendleton wrote:


Hello Nufail,

I think that these methods are going to be challenging to exercise, since
they may only arise due to implementation errors in either the client or
the server.

We have a special test suite that we have used in the past to force the
execution of code paths such as these. You can find that test suite here:

java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/ProtocolTest.java 



and its test data is located here:

java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/protocol.tests 



As a first step, you could try running the ProtocolTest test suite with
code coverage enabled, just to confirm that it doesn't currently exercise
these particular methods in NetConnectionReply.

I believe with ProtocolTest the client is not invoked at all but rather 
replaces the client, so it can only be used to help cover server side 
code.  I put some more details in my response to  this thread.

Re: Code coverage for client.net package

[Katherine Marsden]

On 6/17/2012 1:37 AM, Mohamed Nufail wrote:

Hi,

I looked into some methods with low coverage to get an idea as to how 
to invoke them through a test.
First in case you have not been introduced to the amazing literary 
masterpiece and valuable sleep aid  of the three volume 2000+ page DRDA 
standard, I wanted to point you to that:

https://collaboration.opengroup.org/dbiop/?gpid=453
These are the books that describe the protocol flow between server and 
client



I think chapter 4 of the DRDA manual (volume 1) is worth skimming 
especially trying to understand some of the sample flows as in Figure 
4-2, 4-21, 4-27, and correlating the commands to the DDM manual and 
understanding how to look up specific commands in there. Note Derby does 
not do any of the binding described at length in this chapter.


We also have a couple of good protocol related pages that Bryan put on 
our wiki:

http://wiki.apache.org/db-derby/ProtocolDebuggingTips
http://wiki.apache.org/db-derby/DssProtocolErrors

After doing some reading, try in ij connecting to network server with 
the tracefile attribute, e.g. start network server and connect with:

connect 'jdbc:derby://localhost:1527/wobmat;create=true;traceFile=trace.out'
and then do a simple create table, insert and select and look at the 
trace file. See if you can follow the protocol in the trace file as it 
relates to the JDBC executed.


This will give a good protocol primer to understand how to track down 
ways to cover code and what is important to cover.


parseSQLDCGRP (Sqlca [], int): intin NetConnectionReply
This seems to be called through parseSQLDIAGGRP(Sqlca[] rowsetSqlca). 
But code coverage shows that it returns after executing following if 
statement and it never reaches code below that.


2757if (readFastUnsignedByte() == CodePoint.NULLDATA) {
2758return 0;
2759}


So this would be invoked if we don't have NULLDATA code point. How is 
it possible?


So since the client is parsing what the server is writing, you would 
want to look for the corresponding writeSQLDCGRP method in 
DRDAConnThread.  Is that covered and is it possible that the server 
would ever write anything more than NULLDATA at this point?  If not, is 
it worth keeping the client code beyond this statement?




parseCMDNSPRM (): void   in NetConnectionReply
This would be invoked by parseCommonError(int) in NetConnectionReply 
or parseFetchError(ResultSetCallbackInterface) in NetResultSetReply 
when we have a CMDNSPRM code point. This could happen when 
DRDAConnThread.java calls codePointNotSupported(int codePoint) method 
which is the default case for the switch(codePoint) statement in 
processCommands() method in DRDAConnThread.
So when can we have a code point which is different to the code points 
checked in processCommands()?






parseCMDNSPRM()  this is where the  client would parse a response from 
the server that the client sent invalid protocol, so we don't expect 
that we would ever have to go through that code, yet it makes sense for 
both the server to handle receiving an invalid command and client to 
parse the server's response.   We don't have a good mechanism for 
covering these types of negative protocol tests for the client.



We actually  have a mechanism for covering these types of negative cases 
for the server which is the protocol tests. They are invoked in the test 
org.apache.DerbyTesting.functionTests.tests.derbynet.ProtocolTest but 
the actual protocol being tested is in a file called protocol.tests 
which is a sort of protocol script and simulated client.   You can see 
the checkError lines check for various protocol errors.  (protocol.tests 
is actually worth looking at as part of your DRDA study.) Using this 
mechanism, one could cover the server codePointNotSupported method, but 
obviously it wouldn't help with the client.


I suppose some similar test framework.could be written to throw 
unexpected  protocol at the client to exercise some of the client code 
to detect a misbehaved server but I don't think it is worth it.  In 
addition this particular case is geared to detecting the client gives a 
proper response to to the server's proper response to the client 
misbehaving, so I think it is just one of those cases that ok to leave 
uncovered.


I would prefer to see you focus on code paths for positive cases than 
extremely remote negative protocol errors.


So if looking at NetConnectionReply, I would say focus first on methods 
that are not covered at all and do not sound  protocol error related for 
example, in that file I see parseRDBATHRM which  I would think we would 
cover for an authentication failure and something called 
verifyConnectReply(). Is that called anywhere and what should it do? You 
might want to look at a higher level am.Connection and net.Connection. 
What's not covered there?


HTH

Kathey

Re: Code coverage for client.net package

[Mohamed Nufail]

Hi,

Thank you for the guidance. I will try running ProtocolTest and open JIRA
issues and continue with your suggestions.

Thanks,
Nufail.

On Sun, Jun 17, 2012 at 10:37 PM, Bryan Pendleton <
bpendleton.de...@gmail.com> wrote:

> I looked into some methods with low coverage to get an idea as to how to
>> invoke them through a test.
>>
>> parseSQLDCGRP (Sqlca [], int): intin NetConnectionReply
>> This seems to be called through parseSQLDIAGGRP(Sqlca[] rowsetSqlca). But
>> code coverage shows that it returns after executing
>> following if statement and it never reaches code below that.
>>
>> 2757if (readFastUnsignedByte() == CodePoint.NULLDATA) {
>> 2758return 0;
>> 2759}
>>
>>
>> So this would be invoked if we don't have NULLDATA code point. How is it
>> possible?
>>
>> parseCMDNSPRM (): void   in NetConnectionReply
>> This would be invoked by parseCommonError(int) in NetConnectionReply or
>> parseFetchError(**ResultSetCallbackInterface) in
>> NetResultSetReply when we have a CMDNSPRM code point. This could happen
>> when DRDAConnThread.java calls codePointNotSupported(int
>> codePoint) method which is the default case for the switch(codePoint)
>> statement in processCommands() method in DRDAConnThread.
>> So when can we have a code point which is different to the code points
>> checked in processCommands()?
>>
>> Any idea regarding these tests or how to proceed with improving the
>> coverage would be helpful.
>>
>
> Hello Nufail,
>
> I think that these methods are going to be challenging to exercise, since
> they may only arise due to implementation errors in either the client or
> the server.
>
> We have a special test suite that we have used in the past to force the
> execution of code paths such as these. You can find that test suite here:
>
> java/testing/org/apache/**derbyTesting/functionTests/**
> tests/derbynet/ProtocolTest.**java
>
> and its test data is located here:
>
> java/testing/org/apache/**derbyTesting/functionTests/**
> tests/derbynet/protocol.tests
>
> As a first step, you could try running the ProtocolTest test suite with
> code coverage enabled, just to confirm that it doesn't currently exercise
> these particular methods in NetConnectionReply.
>
> To run just that one test, you can do:
>
> ant -Dderby.junit.testclass=org.**apache.derbyTesting.**
> functionTests.tests.derbynet.**ProtocolTest junit-single
>
> (Or use the emma-single target, of course, to get coverage information)
>
> Then, the next step, is to see if we can add some additional tests into the
> ProtocolTest suite to add this coverage.
>
> We may find that we have to enhance ProtocolTest.java, in addition to
> adding
> more tests in protocol.tests. For example, to specify a code point, the
> test
> suite runs the 'startDdm' statement in protocol.tests, which causes
> ProtocolTest
> to run the getCP() and decodeCP() methods, which appear to currently be
> coded
> to refuse the specification of an unknown code point, so we may have to
> start
> by allowing the tests to specify an unknown code point; then you can add a
> test
> which specifies that unknown code point.
>
> Again, I encourage you to open separate JIRA issues for each of these
> various
> tasks, so that you can record your investigations into the various methods,
> and (eventually) record test patches that you develop.
>
> thanks,
>
> bryan
>



-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Bryan Pendleton]

I looked into some methods with low coverage to get an idea as to how to invoke 
them through a test.

parseSQLDCGRP (Sqlca [], int): intin NetConnectionReply
This seems to be called through parseSQLDIAGGRP(Sqlca[] rowsetSqlca). But code 
coverage shows that it returns after executing
following if statement and it never reaches code below that.

2757if (readFastUnsignedByte() == CodePoint.NULLDATA) {
2758return 0;
2759}


So this would be invoked if we don't have NULLDATA code point. How is it 
possible?

parseCMDNSPRM (): void   in NetConnectionReply
This would be invoked by parseCommonError(int) in NetConnectionReply or 
parseFetchError(ResultSetCallbackInterface) in
NetResultSetReply when we have a CMDNSPRM code point. This could happen when 
DRDAConnThread.java calls codePointNotSupported(int
codePoint) method which is the default case for the switch(codePoint) statement 
in processCommands() method in DRDAConnThread.
So when can we have a code point which is different to the code points checked 
in processCommands()?

Any idea regarding these tests or how to proceed with improving the coverage 
would be helpful.


Hello Nufail,

I think that these methods are going to be challenging to exercise, since
they may only arise due to implementation errors in either the client or
the server.

We have a special test suite that we have used in the past to force the
execution of code paths such as these. You can find that test suite here:

java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/ProtocolTest.java

and its test data is located here:

java/testing/org/apache/derbyTesting/functionTests/tests/derbynet/protocol.tests

As a first step, you could try running the ProtocolTest test suite with
code coverage enabled, just to confirm that it doesn't currently exercise
these particular methods in NetConnectionReply.

To run just that one test, you can do:

ant 
-Dderby.junit.testclass=org.apache.derbyTesting.functionTests.tests.derbynet.ProtocolTest
 junit-single

(Or use the emma-single target, of course, to get coverage information)

Then, the next step, is to see if we can add some additional tests into the
ProtocolTest suite to add this coverage.

We may find that we have to enhance ProtocolTest.java, in addition to adding
more tests in protocol.tests. For example, to specify a code point, the test
suite runs the 'startDdm' statement in protocol.tests, which causes ProtocolTest
to run the getCP() and decodeCP() methods, which appear to currently be coded
to refuse the specification of an unknown code point, so we may have to start
by allowing the tests to specify an unknown code point; then you can add a test
which specifies that unknown code point.

Again, I encourage you to open separate JIRA issues for each of these various
tasks, so that you can record your investigations into the various methods,
and (eventually) record test patches that you develop.

thanks,

bryan

Re: Code coverage for client.net package

[Mohamed Nufail]

Hi,

I looked into some methods with low coverage to get an idea as to how to
invoke them through a test.

parseSQLDCGRP (Sqlca [], int): intin NetConnectionReply
This seems to be called through parseSQLDIAGGRP(Sqlca[] rowsetSqlca). But
code coverage shows that it returns after executing following if statement
and it never reaches code below that.

2757if (readFastUnsignedByte() == CodePoint.NULLDATA) {2758
return 0; 2759}
So this would be invoked if we don't have NULLDATA code point. How is it
possible?

parseCMDNSPRM (): void   in NetConnectionReply
This would be invoked by parseCommonError(int) in NetConnectionReply or
parseFetchError(ResultSetCallbackInterface) in NetResultSetReply when we
have a CMDNSPRM code point. This could happen when DRDAConnThread.java
calls codePointNotSupported(int codePoint) method which is the default case
for the switch(codePoint) statement in processCommands() method in
DRDAConnThread.
So when can we have a code point which is different to the code points
checked in processCommands()?

Any idea regarding these tests or how to proceed with improving the
coverage would be helpful.

thanks,
Nufail.

On Wed, Jun 13, 2012 at 3:27 AM, Dag H. Wanvik wrote:

> Bryan Pendleton  writes:
>
> >> This looks like a great collection of methods to focus on.
> >>
> >> Actually what I meant to say was that these methods are not being
> >> called in anywhere. Code inspection from my IDE revealed that
> >> these methods are unused. So it might be possible to remove them.
> >
> > Even better!
> >
> > Although it is possible to do tricky things like invoking methods
> > via reflection, in general the Derby codebase doesn't do that
> > (there are some important exceptions, but...).
> >
> > So if your IDE believes that these methods aren't being called,
> > I agree that it seems like we should be able to remove them.
>
> I usually do a textual search as well to avoid missing those few that
> are called via reflection. Note that on the server side it is not enough
> to search the Java files, some classes are referenced only via
> .properties files (module implementations). So I usually search
> everything under "java", or, if I am certain it's not in the
> "java/shared" folder, just in "java/engine". But there are even
> exceptions to that: some classes are only used by tests (scaffolding),
> so sometime search all inside all of "java" may be necessary, some stuff
> in "java/build" is only called from Ant build.xml scripts etc.. YMMV.
>
> I think the client side is easier, though: reflection is probably the
> only pit one can fall into there, so textual search in "java/shared" and
> "java/client" should suffice.
>
> Dag
>
>
>
>
> >
> > Let's proceed on the assumption that we'll try to remove these
> > unused and un-called methods, unless we can find some evidence
> > that shows a reason to retain them.
> >
> > thanks,
> >
> > bryan
>



-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Dag H. Wanvik]

Bryan Pendleton  writes:

>> This looks like a great collection of methods to focus on.
>>
>> Actually what I meant to say was that these methods are not being
>> called in anywhere. Code inspection from my IDE revealed that
>> these methods are unused. So it might be possible to remove them.
>
> Even better!
>
> Although it is possible to do tricky things like invoking methods
> via reflection, in general the Derby codebase doesn't do that
> (there are some important exceptions, but...).
>
> So if your IDE believes that these methods aren't being called,
> I agree that it seems like we should be able to remove them.

I usually do a textual search as well to avoid missing those few that
are called via reflection. Note that on the server side it is not enough
to search the Java files, some classes are referenced only via
.properties files (module implementations). So I usually search
everything under "java", or, if I am certain it's not in the
"java/shared" folder, just in "java/engine". But there are even
exceptions to that: some classes are only used by tests (scaffolding),
so sometime search all inside all of "java" may be necessary, some stuff
in "java/build" is only called from Ant build.xml scripts etc.. YMMV.

I think the client side is easier, though: reflection is probably the
only pit one can fall into there, so textual search in "java/shared" and
"java/client" should suffice.

Dag




>
> Let's proceed on the assumption that we'll try to remove these
> unused and un-called methods, unless we can find some evidence
> that shows a reason to retain them.
>
> thanks,
>
> bryan

Re: Code coverage for client.net package

[Bryan Pendleton]

This looks like a great collection of methods to focus on.

Actually what I meant to say was that these methods are not being called in 
anywhere. Code inspection from my IDE revealed that
these methods are unused. So it might be possible to remove them.


Even better!

Although it is possible to do tricky things like invoking methods
via reflection, in general the Derby codebase doesn't do that
(there are some important exceptions, but...).

So if your IDE believes that these methods aren't being called,
I agree that it seems like we should be able to remove them.

Let's proceed on the assumption that we'll try to remove these
unused and un-called methods, unless we can find some evidence
that shows a reason to retain them.

thanks,

bryan

Re: Code coverage for client.net package

[Mohamed Nufail]

On Mon, Jun 11, 2012 at 7:45 PM, Bryan Pendleton  wrote:

>
> This looks like a great collection of methods to focus on.
>

Actually what I meant to say was that these methods are not being called in
anywhere. Code inspection from my IDE revealed that these methods are
unused. So it might be possible to remove them.


>
> I suggest that you open a single JIRA, with a subject something like:
>
>Investigate Network Client methods with zero test coverage
>
> and list these methods in it.
>
> Then, as we explore the individual methods one by one, you can open
> sub-task JIRA issues as children of the first one, to record the
> work that we do for that particular method or methods.
>
> Do you have a particular method that seems interesting to work on first?
>
> I don't have a particular idea on which methods would be better to tackle
first. But this procedure seems to be the way to go.


>
> I agree, this is challenging. What debugger or IDE do you use? I think
> that most IDEs, including Eclipse, NetBeans, and IntelliJ, have some
> tools for finding the callers of a particular method.
>
>
I'm using IntelliJ. But the problem is these methods seem not to be
directly called. I'll try running the debugger to see how they work.


-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Bryan Pendleton]

On 06/11/2012 01:07 AM, Mohamed Nufail wrote:

By analysing those classes, I found that the following methods have no usage at 
all.


This looks like a great collection of methods to focus on.

I suggest that you open a single JIRA, with a subject something like:

Investigate Network Client methods with zero test coverage

and list these methods in it.

Then, as we explore the individual methods one by one, you can open
sub-task JIRA issues as children of the first one, to record the
work that we do for that particular method or methods.

Do you have a particular method that seems interesting to work on first?

> Also I tried the suggested techniques for coming up with tests, but found it 
difficult to determine the top-level classes
> calling the methods in network client.

I agree, this is challenging. What debugger or IDE do you use? I think
that most IDEs, including Eclipse, NetBeans, and IntelliJ, have some
tools for finding the callers of a particular method.

thanks,

bryan

Re: Code coverage for client.net package

[Mohamed Nufail]

Hi,

Thank you everyone for the valuable suggestions.

By analysing those classes, I found that the following methods have no
usage at all.

NetConnectionReply class
verifyConnectReply(int codept)
readDummyExchangeServerAttributes(Connection connection)
checkRequiredObjects(boolean receivedFlag, boolean receivedFlag2, boolean
receivedFlag3, boolean receivedFlag4, boolean receivedFlag5, boolean
receivedFlag6)
checkRequiredObjects(boolean receivedFlag, boolean receivedFlag2, boolean
receivedFlag3, boolean receivedFlag4, boolean receivedFlag5, boolean
receivedFlag6, boolean receivedFlag7)

NetPackageReply class
parsePKGNAMCT(boolean skip)

NetStatementReply class
parseQRYPRCTYP()
parseSQLCSRHLD()
parseQRYATTSCR()
parseQRYATTSET()
parseQRYATTSNS()
parseQRYATTUPD()


Also I tried the suggested techniques for coming up with tests, but found
it difficult to determine the top-level classes calling the methods in
network client.

I hope to look into sql interface implementations on the client to come up
with tests.

Thanks,
Nufail.

Re: Code coverage for client.net package

[Bryan Pendleton]

On 06/06/2012 04:10 AM, Knut Anders Hatlen wrote:

One strategy may be to pick one piece of code that lacks coverage,
and find the corresponding code on the server which sends that kind of
data to the client.


I think this is a good technique.

I tried your suggested technique, looking at the DSCSQLSTT code
point and how it was handled (lines 176-182 of NetStatementReply.java
are apparently not covered).

I got as far as determining that the client code would be executed,
if, on the server side, DRDAStatement::Prepare() should happen to
throw a SQLException.

I then spent some time trying to figure out possible ways to provoke
such an exception to be thrown.

I didn't find such a code path, but I *did* encounter a DRDA protocol
crash, which I was able to log as DERBY-5806.

Thus:
a) I think your suggested technique for trying to increase the coverage
is an excellent one, and
b) Even if it is challenging to provoke these new code paths to be
taken, simply attempting to cause them to occur can be a fruitful
way to find new bugs!

thanks,

bryan

Re: Code coverage for client.net package

[Bryan Pendleton]

I think that the class NetConnectionReply is a good one to investigate.
From what I can see, there are a lot of coverage gaps in this class,
at least according to 
http://dbtg.foundry.sun.com/derby/test/coverage/_files/3c.html

I agree that the protocol errors are hard to test for, so it's
easier to look for other parts of the class that aren't covered
very well.

It's interesting that there appear to be large sections of this
large and and complex class that aren't touched at all. For example,
NetConnectionReply::parseCMDNSPRM() appears to be untouched:
http://dbtg.foundry.sun.com/derby/test/coverage/_files/33a.html#e

And that is a pretty large section of code that we aren't calling at all?

I wonder if perhaps I'm reading the coverage reports incorrectly.

But if not, and if, as Knut Anders suggested, there are some more
opportunities to find entirely dead code that we don't need, that
would be great, as the less code we have, the less we have to maintain.

I think that substantial parts of the Derby client library were originally
descended from a library that spoke to various DB2 server implementations,
and I think that there is possibly a fair amount of the client code that
was needed for the DB2 inter-operability, but is never used when a Derby
client is speaking to a Derby server.

So in this case, for example, can the Derby server ever return a CMDNSPRM
message to the client?

thanks,

bryan

Re: Code coverage for client.net package

[Katherine Marsden]

On 6/6/2012 4:10 AM, Knut Anders Hatlen wrote:
However, it looks like a lot of the code is handling errors, many of 
them protocol errors (which would only happen if there's a bug in the 
server or the client) or very unlikely errors (like 
NetResultSetReply.parseCloseError() which is only called if the server 
experienced an error in ResultSet.close()). It's very hard to test 
this code, and it might even require changes on the server to force it 
to generate errors while testing. It might be easier to start with the 
classes that implement the java.sql interfaces in the client.am 
package, as most of their methods could be called directly from the 
test. I see for example that classes like LogicalPreparedStatement and 
LogicalCallableStatement have very little coverage. 


For protocol errors usually these are only exercised with the protocol 
tests. (By adding them to protocol.tests in the derbynet  package and 
running derbynet.ProtocolTest.  That said, that process can be somewhat 
painful and  I don't think they are high value test cases as really 
although the client needs to handle them, they should not happen in 
normal operation.


I  think for a summer project it is good to focus on method vs block 
coverage and agree it is a higher priority and easier  to get the basic 
sql interface implementation methods covered on the client.


Kathey

Re: Code coverage for client.net package

[Knut Anders Hatlen]

Mohamed Nufail  writes:

> Hi,
>
> Next I will focus on NetConnectionReply, NetPackageReply and
> NetResultSetReply classes of the client.net package which have low
> code coverage. I will look into these classes to get a better
> understanding of them. I would like to know any suggestions regarding
> this.

Hi Nufail,

I think a good first step would be to go through the methods that have
0% coverage and see if they have any callers at all ("find usages" in
your favourite IDE should give a good indication). If they are not
called anywhere in the code, the methods could probably just be removed.

When the obviously dead code is removed, the next step would be to find
out how to exercise the remaining code that's not covered. In these
classes, it will require a lot of detective work and guessing, I think,
as it isn't always easy to see which top-level calls will cause this
low-level code to run.

One strategy may be to pick one piece of code that lacks coverage, for
example this if-statement in NetResultSetReply.parseCNTQRYreply()

172 if (peekCP == CodePoint.ENDQRYRM) {
173 found = true;
174 parseEndQuery(resultSetI);
175 peekCP = peekCodePoint();
176 }

and find the corresponding code on the server which sends that kind of
data to the client. DRDAConnThread is the server class that's
responsible for sending data to the client, and a search for the string
ENDQRYRM in that class will lead you to this code:

759   writeQRYDTA(stmt);
760   if (stmt.rsIsClosed())
761   {
762   writeENDQRYRM(CodePoint.SVRCOD_WARNING);
763   writeNullSQLCARDobject();
764   }

The coverage report says that the writeENDQRYRM() method is never
called, so the question that needs to be answered, is how can
rsIsClosed() return true here. If it's at all possible.

However, it looks like a lot of the code is handling errors, many of
them protocol errors (which would only happen if there's a bug in the
server or the client) or very unlikely errors (like
NetResultSetReply.parseCloseError() which is only called if the server
experienced an error in ResultSet.close()). It's very hard to test this
code, and it might even require changes on the server to force it to
generate errors while testing.

It might be easier to start with the classes that implement the java.sql
interfaces in the client.am package, as most of their methods could be
called directly from the test. I see for example that classes like
LogicalPreparedStatement and LogicalCallableStatement have very little
coverage.

-- 
Knut Anders

Re: Code coverage for client.net package

[Mohamed Nufail]

Hi,

Next I will focus on NetConnectionReply, NetPackageReply and
NetResultSetReply classes of the client.net package which have low code
coverage. I will look into these classes to get a better understanding of
them. I would like to know any suggestions regarding this.

Thanks,
Nufail.

On Sun, May 27, 2012 at 11:55 AM, Mohamed Nufail  wrote:

> I created a JIRA issue for this at
> https://issues.apache.org/jira/browse/DERBY-5786
>
> Regards,
> Nufail.
>
>
> On Sat, May 26, 2012 at 10:03 PM, Mohamed Nufail wrote:
>
>> Will do that. Thanks.
>>
>> Nufail.
>>
>>
>> On Sat, May 26, 2012 at 8:21 PM, Bryan Pendleton <
>> bpendleton.de...@gmail.com> wrote:
>>
>>> So making it a stand alone class and using it instead of InputStreamUtil
 and DynamicByteArrayOutputStream classes should solve
 the problem. Shall I continue with this solution?

>>>
>>> Sure!
>>>
>>> It sounds like the problem is well-enough defined at this point.
>>>
>>> I think the next thing you should do is to open a new JIRA issue
>>> in the Derby JIRA system to track this work. In your JIRA, you
>>> can describe the problem, how you propose to fix it, and include
>>> a reference to this email thread in the mailing list archives.
>>>
>>> Then, once you have coded the solution and are satisfied that it
>>> is working, you can attach your proposed changes to the JIRA as
>>> a patch.
>>>
>>> Here's some information that describes the overall process:
>>> http://wiki.apache.org/db-**derby/DerbyCommitProcess
>>>
>>> thanks,
>>>
>>> bryan
>>>
>>
>>
>>
>> --
>>
>> Mohamed Nufail
>> Undergraduate,
>> Department of Computer Science & Engineering,
>> University of Moratuwa.
>> Blog: http://www.nufailm.blogspot.com/
>>
>>
>>
>
>
> --
>
> Mohamed Nufail
> Undergraduate,
> Department of Computer Science & Engineering,
> University of Moratuwa.
> Blog: http://www.nufailm.blogspot.com/
>
>
>


-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Mohamed Nufail]

I created a JIRA issue for this at
https://issues.apache.org/jira/browse/DERBY-5786

Regards,
Nufail.

On Sat, May 26, 2012 at 10:03 PM, Mohamed Nufail  wrote:

> Will do that. Thanks.
>
> Nufail.
>
>
> On Sat, May 26, 2012 at 8:21 PM, Bryan Pendleton <
> bpendleton.de...@gmail.com> wrote:
>
>> So making it a stand alone class and using it instead of InputStreamUtil
>>> and DynamicByteArrayOutputStream classes should solve
>>> the problem. Shall I continue with this solution?
>>>
>>
>> Sure!
>>
>> It sounds like the problem is well-enough defined at this point.
>>
>> I think the next thing you should do is to open a new JIRA issue
>> in the Derby JIRA system to track this work. In your JIRA, you
>> can describe the problem, how you propose to fix it, and include
>> a reference to this email thread in the mailing list archives.
>>
>> Then, once you have coded the solution and are satisfied that it
>> is working, you can attach your proposed changes to the JIRA as
>> a patch.
>>
>> Here's some information that describes the overall process:
>> http://wiki.apache.org/db-**derby/DerbyCommitProcess
>>
>> thanks,
>>
>> bryan
>>
>
>
>
> --
>
> Mohamed Nufail
> Undergraduate,
> Department of Computer Science & Engineering,
> University of Moratuwa.
> Blog: http://www.nufailm.blogspot.com/
>
>
>


-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Mohamed Nufail]

Will do that. Thanks.

Nufail.

On Sat, May 26, 2012 at 8:21 PM, Bryan Pendleton  wrote:

> So making it a stand alone class and using it instead of InputStreamUtil
>> and DynamicByteArrayOutputStream classes should solve
>> the problem. Shall I continue with this solution?
>>
>
> Sure!
>
> It sounds like the problem is well-enough defined at this point.
>
> I think the next thing you should do is to open a new JIRA issue
> in the Derby JIRA system to track this work. In your JIRA, you
> can describe the problem, how you propose to fix it, and include
> a reference to this email thread in the mailing list archives.
>
> Then, once you have coded the solution and are satisfied that it
> is working, you can attach your proposed changes to the JIRA as
> a patch.
>
> Here's some information that describes the overall process:
> http://wiki.apache.org/db-**derby/DerbyCommitProcess
>
> thanks,
>
> bryan
>



-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Bryan Pendleton]

So making it a stand alone class and using it instead of InputStreamUtil and 
DynamicByteArrayOutputStream classes should solve
the problem. Shall I continue with this solution?


Sure!

It sounds like the problem is well-enough defined at this point.

I think the next thing you should do is to open a new JIRA issue
in the Derby JIRA system to track this work. In your JIRA, you
can describe the problem, how you propose to fix it, and include
a reference to this email thread in the mailing list archives.

Then, once you have coded the solution and are satisfied that it
is working, you can attach your proposed changes to the JIRA as
a patch.

Here's some information that describes the overall process:
http://wiki.apache.org/db-derby/DerbyCommitProcess

thanks,

bryan

Re: Code coverage for client.net package

[Mohamed Nufail]

On Sat, May 26, 2012 at 1:42 AM, Knut Anders Hatlen
wrote:

> Mohamed Nufail  writes:
>
> Hi Nufail,
>
> Thanks for sharing your analysis.
>
> I think you're right that the extra functionality provided by
> PublicBufferOutputStream isn't needed for writeUDT() to function
> correctly.
>
> One small advantage of using PublicBufferOutputStream is that
> PublicBufferOutputStream.getBuffer() returns the buffer directly,
> whereas ByteArrayOutputStream.toByteArray() creates a copy of the
> internal buffer. So by using PublicBufferOutputStream we save one
> allocation of a byte array per write. I would guess this was also the
> motivation for using DynamicByteArrayOutputStream instead of
> ByteArrayOutputStream in the first place.
>
> It's probably not a big deal, but for applications that write many UDTs,
> using PublicBufferOutputStream and accessing the buffer directly might
> take some load off the garbage collector and perform a little better.
>

Yes, that would be a good reason to go with PublicBufferOutputStream.

So making it a stand alone class and using it instead of InputStreamUtil
and DynamicByteArrayOutputStream classes should solve the problem. Shall I
continue with this solution?


>
> --
> Knut Anders
>



-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Bryan Pendleton]

PublicBufferOutputStream.getBuffer() returns the buffer directly,
whereas ByteArrayOutputStream.toByteArray() creates a copy of the
internal buffer. So by using PublicBufferOutputStream we save one
allocation of a byte array per write.


That seems like a strong enough reason to favor PublicBufferOutputStream, to me.

thanks,

bryan

Re: Code coverage for client.net package

[Knut Anders Hatlen]

Mohamed Nufail  writes:

> Hi,
>
> Going through the related Jira issue, I found that the changes have
> been introduced to get proper functionality for UDTs in the network
> client. The network client was required to behave similar to the
> embedded client when the columns contain java objects. For this
> reason, some of the functionality of the engine code was used in the
> client also.
>
> Initially, the two classes InputStreamUtil and
> DynamicByteArrayOutputStream were made common. But due to the
> possibility of failures, they were moved and now there are two copies
> in the engine code and client. But as already mentioned, in the
> network client, only a little part of these are being used in
> Request.writeUDT(). So as Knut mentioned the possible solutions would
> be making use of EncodedInputStream.PublicBufferOutputStream already
> existing in client.net package or using java.io.ByteArrayOutputStream. 
>
> I can't see any advantage of using one over the other except for
> PublicBufferOutputStream giving a direct reference to the byte array.
> But going through writeUDT(), it seems that it is not needed. So will
> it be ok to just use java.io.ByteArrayOutputStream?

Hi Nufail,

Thanks for sharing your analysis.

I think you're right that the extra functionality provided by
PublicBufferOutputStream isn't needed for writeUDT() to function
correctly.

One small advantage of using PublicBufferOutputStream is that
PublicBufferOutputStream.getBuffer() returns the buffer directly,
whereas ByteArrayOutputStream.toByteArray() creates a copy of the
internal buffer. So by using PublicBufferOutputStream we save one
allocation of a byte array per write. I would guess this was also the
motivation for using DynamicByteArrayOutputStream instead of
ByteArrayOutputStream in the first place.

It's probably not a big deal, but for applications that write many UDTs,
using PublicBufferOutputStream and accessing the buffer directly might
take some load off the garbage collector and perform a little better.

-- 
Knut Anders

Re: Code coverage for client.net package

[Mohamed Nufail]

Hi,

Going through the related Jira issue, I found that the changes have been
introduced to get proper functionality for UDTs in the network client. The
network client was required to behave similar to the embedded client when
the columns contain java objects. For this reason, some of the
functionality of the engine code was used in the client also.

Initially, the two classes InputStreamUtil and DynamicByteArrayOutputStream
were made common. But due to the possibility of failures, they were moved
and now there are two copies in the engine code and client. But as already
mentioned, in the network client, only a little part of these are being
used in Request.writeUDT(). So as Knut mentioned the possible solutions
would be making use of EncodedInputStream.PublicBufferOutputStream already
existing in client.net package or using java.io.ByteArrayOutputStream.

I can't see any advantage of using one over the other except for
PublicBufferOutputStream giving a direct reference to the byte array. But
going through writeUDT(), it seems that it is not needed. So will it be ok
to just use java.io.ByteArrayOutputStream?

Regards,
Nufail.

On Wed, May 23, 2012 at 10:53 AM, Mohamed Nufail  wrote:

> Thank you. I will go through those and come up with a solution.
>
> regards,
> Nufail.
>
> On Wed, May 23, 2012 at 9:22 AM, Bryan Pendleton <
> bpendleton.de...@gmail.com> wrote:
>
>> If only such little functionality is utilized, having those two classes
>>> is a waste. The new solution sounds much better.
>>>
>>> So shall I proceed with the suggested solution?
>>>
>>
>> Yes, I think that would be great!
>>
>> As Knut Anders observed, these two classes were created
>> fairly recently.
>>
>> You might start by reviewing the original change that
>> created these two files, and the JIRA entry that tracked
>> the discussion around that change, so that you have all
>> the background as to why those classes were created in
>> that fashion.
>>
>> Then you could explore the options that Knut Anders suggested,
>> or perhaps after your studies you will have a different
>> solution to propose.
>>
>> I believe that the change/JIRA information to examine is this:
>>
>> $ svn log InputStreamUtil.java
>> --**--**
>> 
>> r899819 | rhillegas | 2010-01-15 13:32:14 -0800 (Fri, 15 Jan 2010) | 1
>> line
>>
>> DERBY-4491: Fix sealing violation which kills the AssertFailureTest on
>> runs against insane jar files.
>> --**--**
>> 
>>
>> We'll be looking forward to hear what you learn as you study this.
>>
>> thanks,
>>
>> bryan
>>
>>
>
>
> --
>
> Mohamed Nufail
> Undergraduate,
> Department of Computer Science & Engineering,
> University of Moratuwa.
> Blog: http://www.nufailm.blogspot.com/
>
>
>


-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Mohamed Nufail]

Thank you. I will go through those and come up with a solution.

regards,
Nufail.

On Wed, May 23, 2012 at 9:22 AM, Bryan Pendleton  wrote:

> If only such little functionality is utilized, having those two classes is
>> a waste. The new solution sounds much better.
>>
>> So shall I proceed with the suggested solution?
>>
>
> Yes, I think that would be great!
>
> As Knut Anders observed, these two classes were created
> fairly recently.
>
> You might start by reviewing the original change that
> created these two files, and the JIRA entry that tracked
> the discussion around that change, so that you have all
> the background as to why those classes were created in
> that fashion.
>
> Then you could explore the options that Knut Anders suggested,
> or perhaps after your studies you will have a different
> solution to propose.
>
> I believe that the change/JIRA information to examine is this:
>
> $ svn log InputStreamUtil.java
> --**--**
> 
> r899819 | rhillegas | 2010-01-15 13:32:14 -0800 (Fri, 15 Jan 2010) | 1 line
>
> DERBY-4491: Fix sealing violation which kills the AssertFailureTest on
> runs against insane jar files.
> --**--**
> 
>
> We'll be looking forward to hear what you learn as you study this.
>
> thanks,
>
> bryan
>
>


-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Bryan Pendleton]

If only such little functionality is utilized, having those two classes is a 
waste. The new solution sounds much better.

So shall I proceed with the suggested solution?


Yes, I think that would be great!

As Knut Anders observed, these two classes were created
fairly recently.

You might start by reviewing the original change that
created these two files, and the JIRA entry that tracked
the discussion around that change, so that you have all
the background as to why those classes were created in
that fashion.

Then you could explore the options that Knut Anders suggested,
or perhaps after your studies you will have a different
solution to propose.

I believe that the change/JIRA information to examine is this:

$ svn log InputStreamUtil.java

r899819 | rhillegas | 2010-01-15 13:32:14 -0800 (Fri, 15 Jan 2010) | 1 line

DERBY-4491: Fix sealing violation which kills the AssertFailureTest on runs 
against insane jar files.


We'll be looking forward to hear what you learn as you study this.

thanks,

bryan

Re: Code coverage for client.net package

[Mohamed Nufail]

+1

If only such little functionality is utilized, having those two classes is
a waste. The new solution sounds much better.

So shall I proceed with the suggested solution?

Regards,
Nufail.

On Wed, May 23, 2012 at 5:22 AM, Bryan Pendleton  wrote:

> On 05/22/2012 07:51 AM, Knut Anders Hatlen wrote:
>
>> I'm wondering if it would be more reasonable just to remove the two
>> classes and make writeUDT() use either java.io.ByteArrayOutputStream or
>> EncodedInputStream.**PublicBufferOutputStream instead. The latter class
>> already is in the client.net package, but it would probably be a good
>> idea to make it a stand-alone class and not an inner class if we want to
>> reuse it in the Request class.
>>
>> That would reduce the code size and increase the percentage covered.
>>
>
> +1
>
> This sounds like a great idea to me. Nufail, what do you think?
>
> thanks,
>
> bryan
>
>


-- 

Mohamed Nufail
Undergraduate,
Department of Computer Science & Engineering,
University of Moratuwa.
Blog: http://www.nufailm.blogspot.com/

Re: Code coverage for client.net package

[Bryan Pendleton]

On 05/22/2012 07:51 AM, Knut Anders Hatlen wrote:

I'm wondering if it would be more reasonable just to remove the two
classes and make writeUDT() use either java.io.ByteArrayOutputStream or
EncodedInputStream.PublicBufferOutputStream instead. The latter class
already is in the client.net package, but it would probably be a good
idea to make it a stand-alone class and not an inner class if we want to
reuse it in the Request class.

That would reduce the code size and increase the percentage covered.


+1

This sounds like a great idea to me. Nufail, what do you think?

thanks,

bryan

Re: Code coverage for client.net package

[Knut Anders Hatlen]

Bryan Pendleton  writes:

>> First I will focus on the client.net  package. In the 
>> schedule I mentioned that I would be looking into the
>> InputStreamUtil and DynamicByteArrayOutputStream classes first, because I 
>> thought they would be easy to tackle with initially
>> and those two classes have the least coverage in that package according to 
>> emma reports.
>
> This sounds like a fine approach.
>
> I wonder: is it possible that we have existing tests that cover
> many of these classes, but are not being run because we currently
> run those tests only in the embedded configuration, and not
> in the network client configuration?

Those two classes have been copied from the engine code over to the
client, but the client uses only a small part of the functionality they
provide (in Request.writeUDT()).

I'm wondering if it would be more reasonable just to remove the two
classes and make writeUDT() use either java.io.ByteArrayOutputStream or
EncodedInputStream.PublicBufferOutputStream instead. The latter class
already is in the client.net package, but it would probably be a good
idea to make it a stand-alone class and not an inner class if we want to
reuse it in the Request class.

That would reduce the code size and increase the percentage covered.
Sounds like a win-win! :)

> Or, related, is it possible that our current Emma configuration
> is such that we are only measuring one side of the client-server
> pair in a test run?

I think it's measuring both sides. Most of the client/server tests run
with the client and the server in the same JVM process, so at least
those tests should be covered, and I think the test framework also
propagates the EMMA flags to spawned processes.

> Perhaps someone who has worked through the details of the
> Emma configuration for our test runs can make an educated guess
> about whether either of these questions are worth pursuing?
>
> thanks,
>
> bryan

-- 
Knut Anders

Re: Code coverage for client.net package

[Bryan Pendleton]

First I will focus on the client.net  package. In the 
schedule I mentioned that I would be looking into the
InputStreamUtil and DynamicByteArrayOutputStream classes first, because I 
thought they would be easy to tackle with initially
and those two classes have the least coverage in that package according to emma 
reports.


This sounds like a fine approach.

I wonder: is it possible that we have existing tests that cover
many of these classes, but are not being run because we currently
run those tests only in the embedded configuration, and not
in the network client configuration?

Or, related, is it possible that our current Emma configuration
is such that we are only measuring one side of the client-server
pair in a test run?

Perhaps someone who has worked through the details of the
Emma configuration for our test runs can make an educated guess
about whether either of these questions are worth pursuing?

thanks,

bryan