git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Shawn O. Pearce" <spearce@spearce.org>
To: imyousuf@gmail.com
Cc: git@vger.kernel.org, robin.rosenberg@dewire.com,
	Imran M Yousuf <imyousuf@smartitengineering.com>
Subject: Re: [JGit Patch v2 4/7] Use test resources from classpath
Date: Thu, 21 Aug 2008 08:19:04 -0700	[thread overview]
Message-ID: <20080821151904.GT3483@spearce.org> (raw)
In-Reply-To: <1219288394-1241-4-git-send-email-imyousuf@gmail.com>

imyousuf@gmail.com wrote:
> From: Imran M Yousuf <imyousuf@smartitengineering.com>
> 
> Searched and fixed usage of resources, all tests are now passing and using
> classpath resources.
> 
> A utility class for test classes are created. One of its operation turns
> classpath resources to File and it is used by all test classes to locate
> test resources.

I would change the way you do this series a little bit.

Start the series with this patch, only have JGitTestUtil do:

	return new File("test", name);

like DirCacheCGitCompatabilityTest.pathOf does.  That way you
have the code replacement done in the first step, before you start
mucking around with the resource locations.

In the 2nd patch, change JGitTestUtil to get resources from
the classpath, move (not copy) the resources, and add the
tst-rsc directory to the Eclipse .classpath file.

In the 3rd patch, add your Maven POM file, including the .gitignore
for "target".

The tests will always work, and we don't get this weird copy-delete
pair on the resources.

> +/**
> + *
> + * @author imyousuf
> + */
> +public abstract class JGitTestUtil {

We don't comment classes like this.  Either document it for real,
or remove the Javadoc comment entirely.  In "real" documentation
we do not include @author tags.

> +    public static final String CLASSPATH_TO_RESOURCES =
> +        "/org/spearce/jgit/test/resources/";
> +    private JGitTestUtil() {
> +        throw new AssertionError();
> +    }
> +    
> +    public static File getTestResourceFile(String fileName) {
> +        if(fileName == null || fileName.length() <= 0) {

Formatting error.  We put space between "if(".

> +            return null;
> +        }
> +        URL url = JGitTestUtil.class.getResource(
> +            new StringBuilder(CLASSPATH_TO_RESOURCES)
> +                .append(fileName).toString());
> +        return new File(url.getPath());
> +    }
>  }

-- 
Shawn.

      parent reply	other threads:[~2008-08-21 15:20 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-08-21  3:13 [JGit Patch v2 1/7] Add a POM file for setting JGit library as a Maven project imyousuf
2008-08-21  3:13 ` [JGit Patch v2 2/7] Add test resources to a new package structured directory imyousuf
2008-08-21  3:13   ` [JGit Patch v2 3/7] Add test resources directory as a classpath entry imyousuf
2008-08-21  3:13     ` [JGit Patch v2 4/7] Use test resources from classpath imyousuf
2008-08-21  3:13       ` [JGit Patch v2 5/7] Add script for adding second pack for test purpose imyousuf
     [not found]         ` <1219288394-1241-6-git-send-email-imyousuf@gmail.com>
2008-08-21  3:13           ` [JGit Patch v2 7/7] Add ignore list for mavenized JGit imyousuf
2008-08-21 15:12         ` [JGit Patch v2 5/7] Add script for adding second pack for test purpose Shawn O. Pearce
2008-08-22  2:52           ` Imran M Yousuf
2008-08-22 16:06             ` Shawn O. Pearce
2008-08-23  5:21               ` Imran M Yousuf
2008-08-21 15:19       ` Shawn O. Pearce [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20080821151904.GT3483@spearce.org \
    --to=spearce@spearce.org \
    --cc=git@vger.kernel.org \
    --cc=imyousuf@gmail.com \
    --cc=imyousuf@smartitengineering.com \
    --cc=robin.rosenberg@dewire.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).