On Sun, Jun 14, 2015 at 1:42 PM, Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> wrote: > Karthik Nayak <karthik.188@xxxxxxxxx> writes: > >> +test_expect_success 'setup some history and refs' ' >> + test_commit one && >> + test_commit two && >> + test_commit three && >> + git checkout -b side && >> + test_commit four && >> + git checkout master && >> + git update-ref refs/odd/spot master > > I think you want more corner-cases here. For example, > refs/headsfoo/master should not match refs/heads, and it's easy to write > an incorrect implementation for this => test. Like you mentioned below, currently it's redundant with t6300 and its purpose is to add tests for new functionality we provide to for-each-ref via ref-filter. This is done in the next few patches in this series. > >> +' >> +test_expect_success 'filtering by leading name' ' > > Blank line between tests please. Noted. Thanks. > >> + cat >expect <<-\EOF && >> + refs/heads/master >> + refs/heads/side >> + EOF >> + git for-each-ref --format="%(refname)" refs/heads >actual && >> + test_cmp expect actual >> +' > > Isn't this test redundant with the content of t6300-for-each-ref.sh? > > At this point, you've done only internal refactoring, so you shouldn't > need new tests (except to fix coverage holes in existing tests). > > I guess you're adding this to build more tests on top, but the commit > message should clarify this. > I'll add a note to the commit message about how this test file is so that we can integrate more tests in the future for functionality given to for-each-ref via ref-filter APIs. -- Regards, Karthik Nayak -- 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