Re: [PATCH v2 2/4] fsmonitor: Don't bother pretty-printing JSON from watchman

From: Ben Peart <peartben@gmail.com>
Date: Oct 27, 2017, 03:36 UTC
Message-ID: <11f3e539-1a4f-0572-a712-9f1c50359389@gmail.com>
In-Reply-To: <alpine.DEB.2.10.1710261427300.9817@alexmv-linux>

On 10/26/2017 5:29 PM, Alex Vandiver wrote:

Show 23 quoted lines

> On Thu, 26 Oct 2017, Ben Peart wrote:
>> On 10/25/2017 9:31 PM, Alex Vandiver wrote:
>>> diff --git a/t/t7519/fsmonitor-watchman b/t/t7519/fsmonitor-watchman
>>> index a3e30bf54..79f24325c 100755
>>> --- a/t/t7519/fsmonitor-watchman
>>> +++ b/t/t7519/fsmonitor-watchman
>>> @@ -50,7 +50,7 @@ launch_watchman();
>>>      sub launch_watchman {
>>>    -	my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j')
>>> +	my $pid = open2(\*CHLD_OUT, \*CHLD_IN, 'watchman -j --no-pretty')
>>
>> Since this is a test script performance isn't critical.  This version of the
>> integration script logs the response to a file in .git/watchman-response.json
>> and is much more human readable without the "--no-pretty."  As such, I'd leave
>> this one pretty.
> 
> This would be the first delta between the test file and the template
> file.  It seems quite important to me to attempt to ensure that we're
> testing the _same_ contents that we're suggesting users set up.  In
> fact, it makes more sense to me to just turn this into a symlink to the
> sample template.
>   - Alex
>

If you look closer (actually diff the two files) you will see that the test version includes logging that the sample doesn't include. I found the logging very helpful during testing of the feature and debugging Watchman issues but don't want end users to pay the performance penalty of the logging.

t/t7519/fsmonitor-watchman isn't actually used by any of the automated tests as they can't assume Watchman is available. It is just provided as a convenience to enable manual testing and debugging.