db-derby-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bryan Pendleton (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (DERBY-6315) Improve test coverage of org.apache.derby.impl.io.InputStreamFile
Date Sun, 08 Sep 2013 17:57:52 GMT

    [ https://issues.apache.org/jira/browse/DERBY-6315?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13761494#comment-13761494
] 

Bryan Pendleton commented on DERBY-6315:
----------------------------------------


Hi Ahsan,

I agree with you, the length() method is a bit of a puzzle.

Cracking this puzzle open may require some research, and some experimentation.

First, let's look at the class DirFile, in o.a.d.impl.io.DirFile.java

The first thing to notice is the inheritance hierarchy here is complicated:

    class DirFile extends File implements StorageFile

Now, the StorageFile interface contains a length() method:

    /**
     * Returns the length of the named file if it is not a directory. The return value is
not specified
     * if the file is a directory.
     *
     * @return The length, in bytes, of the named file if it exists and is not a directory,
     *         0 if the file does not exist, or any value if the named file is a directory.
     */
    public long length();

And the File class in the JDK *also* contains a length() method:
 
	public long length()

	Returns the length of the file denoted by this abstract pathname.
	The return value is unspecified if this pathname denotes a directory.

And the DirFile class doesn't contain an *implementation* of the length()
method, so it must be the case that DirFile.length() is actually
implemented by java.io.File.length().

Furthermore, at line 197 of DirFile.java, DirFile.getExclusiveFileLock()
actually calls the length() method:

            if (createNewFile())
            {
                validExclusiveLock = true;
            }
            else if (length() > 0)
            {
                validExclusiveLock = true;
            }

But this code isn't calling the length() method via the StorageFile
interface; it is just invoking the class's own length() method
(implemented by File.length() ) directly.

So I am left with a big question: who calls StorageFile.length()?

I tried an experiment:
1) I removed the length() method from the classes StorageFile,
   InputStreamFile, and JarDBFile.
2) I also removed the length() method from the class CorruptFile.java
   in java/testing/org/apache/derbyTesting/functionTests/util/corruptio
3) I did 'ant clobber' and 'ant clean' and 'ant all'.

Everything compiles and builds just fine.

This is somewhat puzzling to me; it suggests that the StorageFile.length
method is entirely unnecessary, and it could be removed, together
with removing the implementations in InputStreamFile and JarDBFile.

The next thing I tried was to look at the Subversion history for
StorageFile.java. There is not a lot of information here; the length()
method was in the original version of StorageFile.java contributed
to Apache by IBM back in 2004, and hasn't been touched since.

There is a comment in the top of StorageFile.java:

 * This interface abstracts file naming. Any method in this interface
 * that also appears in the java.io.File class should behave as the java.io.File method does.

So that comment explains why the length() method in StorageFile is
implemented to behave just as the length() method in java.io.File.

But it doesn't explain why the StorageFile.length() method is there
in the first place, I don't think.

So where does this leave us? I'm not sure.

I *think* that it would be possible to remove the StorageFile.length()
method; it may have been a leftover from the old Cloudscape implementation
and may simply have not been cleaned up when the Cloudscape code became
open source.

I'll post a message to derby-dev asking for help from some of the
Derby developers who are familiar with the code from that time period.

In the meantime, Ahsan, can you please try to reproduce my experiment,
and let me know if you can confirm my results?

Also, if you are able to build successfully with StorageFile.length()
removed, can you try running some of the test suites? Say:
    org.apache.derbyTesting.functionTests.tests.store._Suite
and
    org.apache.derbyTesting.functionTests.tests.lang._Suite
and
    org.apache.derbyTesting.functionTests.tests.memorydb._Suite

If those tests all pass with StorageFile.length() removed, that would
be additional evidence that this method is no longer needed by the
current Derby codebase.

thanks,

bryan


                
> Improve test coverage of org.apache.derby.impl.io.InputStreamFile
> -----------------------------------------------------------------
>
>                 Key: DERBY-6315
>                 URL: https://issues.apache.org/jira/browse/DERBY-6315
>             Project: Derby
>          Issue Type: Sub-task
>          Components: JDBC
>            Reporter: ahsan shamsudeen
>            Assignee: ahsan shamsudeen
>            Priority: Minor
>              Labels: gsoc2013
>
> According to code coverage analysis, org.apache.derby.impl.io.InputStreamFile has a poor
code coverage.
> This task is to investigate this class and add regression test that exercise the code,
as appropriate. The current coverage report of the class can be found at  http://dbtg.foundry.sun.com/derby/test/coverage/_files/9a.html

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Mime
View raw message