[PATCH] Enhance tests.

Jim Meyering jim at meyering.net
Mon Oct 4 16:36:06 UTC 2010


Jeff Darcy wrote:

> On 10/04/2010 07:32 AM, Jim Meyering wrote:
>> Jeff Darcy wrote:
>> ...
>>> Removed "-v" from iwhd command line.
>>
>> Actually, please don't remove -v (--verbose)
>> from the test's iwhd invocation.
>>
>> I've just encountered one instance where using -v there
>> made iwhd print a diagnostic that saved me some time.
>> I was following attributes-related examples from
>> doc/warehouse_api_final.md and tried to set an attribute
>> without an operator.  The --verbose output informed me of
>> the error (I'll update the API doc) and probably saved me
>> from having to look at the code or use a debugger.
>
> Set an attribute without an operator?  Do you mean the "-X PUT" for a
> single-attribute operation (which is mentioned in the main body of the
> doc), or "-d key=myfile" for the multi-attribute form (which is
> mentioned in Appendix A), or something else?

This is the one that didn't seem to work:

        $ curl -d color="blue" -d flavor="lemon" \
          http://fserver-1:9090/my_bucket/file1/attrs

>> Perhaps that is an argument for converting some DPRINTF diagnostics
>> (which print only when "verbose" is set) to be unconditional,
>> If we err on the side of verbosity for now, we'll get more diagnostics
>> in the t/*.log files, and that's almost always a good thing, especially
>> in early development.
>
> On most of the projects I've worked on in the past, one of the first
> things I've done is implement a log macro/function that lets the user on
> extra logging in individual components or code paths from the command
> line.  I'd greatly prefer a patch to make logging more tunable, rather
> than switching individual messages from one side to another of a single
> "verbose" distinction.

Of course, that would be even better, but something minimal
might be good enough for the moment.  Whichever way you
prefer is fine as long as diagnostics like
the "... is MISSING" ones are printed to the t/*.log file.

> I've attached a much shorter patch incorporating all of Jim's helpful
> suggestions.  I'd like to make one further amendment, but figured I'd
> run it by you guys first.  I consider it bad practive to alias an
> existing/function program without reason.  Sure, it's useful to
> intercept calls in code we don't control, or to minimize changes in
> "settled" code, but neither concern applies here.  I'd prefer that the
> function in init.cfg be named "curl_Q" or some such to distinguish calls
> through it from raw calls which we might also need/want to use some day.

My motivation for usurping the "curl" command name completely
was that none of our tests should be using curl without -q:

    # Always use curl's -q option, so that a user's .rc settings
    # cannot perturb these tests.
    curl() { env curl -q "$@"; }

AFAICS, the same holds for -s -S.  If some case arises in which
we require curl diagnostic output, we can adapt, by defining e.g.,

    bare_curl() { env curl -q "$@"; }

but I would wonder if we really should be tying expected output
to the diagnostics of a particular tool.  What if we want/need
to switch to wget for some reason?  2 or 3 years ago, curl
had numerous problems with robustness that wget did not share.

The idea is to make the most common usage unannotated,
as long as the option set is sufficiently general-purpose.

>  Note that curl_H in t/basic already does the right thing in this
> regard, so we're not even being consistent.

curl_H is a special case, since it encapsulates the
"-H ...relatively-long-option" and its value changes as a
function of a loop variable.
Perhaps you see now why the existing set-up is sensible.

> Would anyone object to such a change?

I'd prefer to see a case that requires curl *without* -s -S first.

> Subject: [PATCH] Enhance tests.
>
> Added failure check for "PUT to root" test.
> Added "-s -S" to curl commands to kill silly progress bars.
> Added rdbuf call in meta.cpp to silence C++ code.

ACK.  Thanks again.


More information about the iwhd-devel mailing list