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

From: Shawn O. Pearce <spearce@spearce.org>
Date: Sep 21, 2009, 19:30 UTC
Message-ID: <20090921193049.GL14660@spearce.org>
In-Reply-To: <1253062116-13830-2-git-send-email-mr.gaffo@gmail.com>

mr.gaffo@gmail.com wrote:

Show 7 quoted lines

> 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.