Re: [PATCH v3] Isolate If-Modified-Since handling in gitweb

[Jakub Narebski <jnareb@gmail.com>]

W. Trevor King wrote:
> On Wed, Mar 21, 2012 at 12:22:44PM -0700, Junio C Hamano wrote:
> > "W. Trevor King" <wking@drexel.edu> writes:

> > > > I think it would be better to add initial tests with refactoring, and
> > > > snapshot specific tests with snapshot support, e.g.:
> > > > 
> > > >   1/2: gitweb: Refactor If-Modified-Since handling and add tests
> > > >   2/2: gitweb: Add If-Modified-Since support for snapshots
> > >
> > > But the new tests would be for the new functionality (i.e. snapshot
> > > support), so they wouldn't belong in the general refactoring commit.
> > 
> > Then you are planning to split it in a wrong way.
> > 
> > ... add tests for codepaths that use i-m-s to make sure your
> > refactoring did not break them...
> 
> Ah, I was assuming that some current tests might be checking the
> current behavior, and that my new tests would be testing my new
> snapshot behavior.  If the old i-m-s handling also needs tests, that
> should happen before any of my previously proposed patches:
> 
> 1: tests for i-m-s and git_feed
> 2: refactor i-m-s handling

Those two can be in single commit.  Tests added need only test i-m-s
using git_feed ('atom' or 'rss' action), as it is the only user,
and you touch only i-m-s handling.

> 3: tests for i-m-s and git_snapshot (which fail until 4)
> 4: add i-m-s to git_snapshot

We usually put tests together with feature.  Tests before feature means
that you would need to mark them as test_expect_failure, as they would
not pass before feature is added, isn't it?

-- 
Jakub Narebski
Poland