Re: [PATCH v5 1/2] gitweb: check given hash before trying to create snapshot

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 26 Sep 2009, Mark Rada wrote:

> Makes things nicer in cases when you hand craft the snapshot URL but
> make a typo in defining the hash variable (e.g. netx instead of next);
> you will now get an error message instead of a broken tarball.
> 
> Tests for t9501 are included to demonstrate added functionality.
> 
> Signed-off-by: Mark Rada <marada@xxxxxxxxxxxx>
> ---

Very nice, and I think robust solution.

Acked-by: Jakub Narebski <jnareb@xxxxxxxxx>

[...]
> diff --git a/t/t9501-gitweb-standalone-http-status.sh b/t/t9501-gitweb-standalone-http-status.sh
> index d0ff21d..0688a57 100644
> --- a/t/t9501-gitweb-standalone-http-status.sh
> +++ b/t/t9501-gitweb-standalone-http-status.sh

BTW. the rest of test scripts are executable, but not this one? Why?
(But correcting this should be done, if needed, in separate commit).

> @@ -75,4 +75,43 @@ test_expect_success \
>  test_debug 'cat gitweb.output'
>  
>  
> +# ----------------------------------------------------------------------
> +# snapshot hash ids
> +
> +test_expect_success 'snapshots: good tree-ish id' '
> +	gitweb_run "p=.git;a=snapshot;h=master;sf=tgz" &&
> +	grep "Status: 200 OK" gitweb.output
> +'
> +test_debug 'cat gitweb.output'
> +

What *could* be improved (but don't *need to*) is to check also HTTP
status and not only formatted error message:

> +test_expect_success 'snapshots: bad tree-ish id' '
> +	gitweb_run "p=.git;a=snapshot;h=frizzumFrazzum;sf=tgz" &&
  +	grep "Status: 404 Not Found" gitweb.output &&
> +	grep "404 - Object does not exist" gitweb.output
> +'
> +test_debug 'cat gitweb.output'

And similarly in other cases.  But it is not something required.
I think what it is now is good enough.

-- 
Jakub Narebski
Poland
--
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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]