Re: [PATCH v7 3/3] gitweb: add If-Modified-Since handling to git_snapshot().

From: Jakub Narebski <jnareb@gmail.com>
Date: Mar 28, 2012, 21:28 UTC
Message-ID: <201203282328.08876.jnareb@gmail.com>
In-Reply-To: <20120328183712.GA5992@odin.tremily.us>

On Wed, 28 Mar 2012, W. Trevor King wrote:

> On Wed, Mar 28, 2012 at 07:11:31PM +0100, Jakub Narebski wrote:

[...]

Show 13 quoted lines

> > And it was not caught by test because CGI.pm can output the last modified
> > header as "Last-modified" (RFC 2616, sec 4.2 states "Field names are
> > case-insensitive"), so the last check should be
> > 
> >   +	! grep -i "Last-Modified" gitweb.output
> > 
> > Hmmm... why we use gitweb.output and not gitweb.headers?  Is it consistency
> > with earlier tests?
> 
> Yes, but I can switch to `gitweb.headers` if you'd like.  Should I
> adjust all the header tests in t9501 to use `gitweb.headers` and `grep
> -i`?  It should probably be a separate patch for the tests that
> existed before my i-m-s additions.

Eh, don't worry about this. First, I think we can assume that HTTP headers from CGI.pm will all start with capital letter.

Second, for positive match being overly strict is safe - if assumption doesn't hold we would get false failure. The problem is for negative match - being overly strict means that we won't catch the breakage.

I think that the gitweb.output vs gitweb.headers (and gitweb.body) is because those tests predate gitweb_run producing gitweb.headers file. Be consistent if you want, or use new feature in new test; you don't need to modernize t9501.

-- 
Jakub Narebski
Poland