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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



mr.gaffo@xxxxxxxxx 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]