Re: [PATCH JGit 1/5] adding tests for ObjectDirectory

["Shawn O. Pearce" <spearce@spearce.org>]

mr.gaffo@gmail.com wrote:
> diff --git a/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java b/org.spearce.jgit.test/tst/org/spearce/jgit/lib/ObjectDirectoryTest.java
> +	private File testDir;
> +
> +	@Override
> +	protected void setUp() throws Exception {
> +		testDir = new File(new File(System.getProperty("java.io.tmpdir")), UUID.randomUUID().toString());
> +	}

Can't we use the same logic we use in RepositoryTestCase to create
the temporary directory for this test?  I would rather keep the
temporary space under target/ when testing under Maven, as it
makes it far easier to clean up the directory.  Plus we know we
have sufficient write space there.

> +	@Override
> +	protected void tearDown() throws Exception {
> +		if (testDir.exists()){

Style nit: Space between ) and {

> +	public void testExistsWithNonExistantDirectory() throws Exception {
> +		assertFalse(new ObjectDirectory(new File("/some/nonexistant/file")).exists());

Please create a path name below your testDir which you know won't
exist.  I don't want this test to rely upon the fact that some
absolute path doesn't exist that is outside of our namespace control.

> +	private void createTestDir(){

You use this method once, inline it inside
testExistsWithExistingDirectory().

Otherwise, the test case is OK, but is still quite sparse with
regards to functionality of the class being tested.  Was it your
intention to only cover the most basic parts at this time?  Its more
coverage than we have now, so I'm happy, but just wanted to point
out it certainly isn't complete (e.g. no pack support).

-- 
Shawn.