Re: [PATCH v5 2/3] gitweb: refactor If-Modified-Since handling

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

On Tue, Mar 27, 2012 at 11:24:20PM +0100, Jakub Narebski wrote:

quoted

On Mon, 26 Mar 2012, W. Trevor King wrote:

quoted

+# ----------------------------------------------------------------------
+# modification times (Last-Modified and If-Modified-Since)
+
+test_expect_success 'modification: feed last-modified' '
+	gitweb_run "p=.git;a=atom;h=master" &&
+	grep "Status: 200 OK" gitweb.output &&
+	grep "Last-modified: Thu, 7 Apr 2005 22:14:13 +0000" gitweb.output
+'

All right.

What's that date from?  Wouldn't it be better to read it from commit
object with `git show -s --pretty=%cD HEAD` or postprocessed from
'%ct' timestamp?

That's the date set by the first `test_tick`, which is hardcoded in
`test-lib-functions.sh`.  Extracting the date dynamically seems
unnecessary, since I can't imagine anyone changing the `test_tick`
date.  

Ah, it's all right then.  I should have checked the test_tick function.

That of course assuming that nobody would add test_tick earlier, but
if he/she does, he/she can deal with fallout...

It's easy enough to do if you think it is appropriate though… 

No, it is not needed.

quoted

quoted

+test_debug 'cat gitweb.headers'
+
+test_expect_success 'modification: feed if-modified-since (modified)' '
+	export HTTP_IF_MODIFIED_SINCE="Wed, 6 Apr 2005 22:14:13 +0000" &&
+	gitweb_run "p=.git;a=atom;h=master" &&
+	unset HTTP_IF_MODIFIED_SINCE &&
+	grep "Status: 200 OK" gitweb.output
+'

I think it *might* be better solution to use test_when_finished:

  +test_expect_success 'modification: feed if-modified-since (modified)' '
  +	export HTTP_IF_MODIFIED_SINCE="Wed, 6 Apr 2005 22:14:13 +0000" &&
  +	test_when_finished "unset HTTP_IF_MODIFIED_SINCE" &&
  +	gitweb_run "p=.git;a=atom;h=master" &&
  +	grep "Status: 200 OK" gitweb.output
  +'

I don't think we need sane_unset here.

Good point.  Sloppy me not reading `t/README` thoroughly enough ;).

-- 
Jakub Narebski
Poland