Re: [PATCH] gitweb: Try harder in parse_tag; perhaps it was given ambiguous name

[Junio C Hamano <gitster@pobox.com>]

Jakub Narebski <jnareb@gmail.com> writes:

> Well, actually that is even better idea. We can go for one of the three
> levels of HTTP status checking:
>
> 1. Check if we got "Status: 200 OK" when we expect it, and not have it
>    when we expect other HTTP status, e.g. when requesting nonexistent
>    file. The above code is enough for that.
>
> 2. We can check if we got expected status number, for example 200 for
>    when we expect no error, or 404 when object is not found, or 403
>    if there is no such object etc. I was thinking about using this version
>    the need to check not full first line, but fragment of it.
>
> 3. We can check full first line, for example
>      Status: 200 OK
>      Status: 403 Forbidden
>      Status: 404 Not Found
>      Status: 400 Bad Request
>    but this might tie gitweb test too tightly with minute details of
>    gitweb output. The above code is good for that too.
>
> What do you think, which route we should go in test?

4. We should check for what we expect in the parts of gitweb output we
   care about.  E.g.

   * If we are making sure it refuses to serve incorrect request
     (e.g. no such repository), we should check for the status, which is
     what we care about (we may not care about how the actual error
     message is stated).

   * Otherwise (and I suspect this is "most of the tests"), we obviously
     expect to have "200 OK", and check for that;

   BUT IN ADDITION

   * If we want to make sure that a specific aspect of the output was
     buggy and a patch fixed it (e.g. an href used a short refname
     without having refs/heads or refs/tags prefixed, causing
     ambiguity), we also should check that part of output in generated
     HTML, not just the HTTP status header.