Patchwork [3,of,9,stable] tests: don't rely on broken behaviour in test-largefiles-cache.t

login
register
mail settings
Submitter Mads Kiilerich
Date Feb. 26, 2013, 2:41 a.m.
Message ID <c92e62e6b93838c00c74.1361846483@mk-desktop>
Download mbox | patch
Permalink /patch/1050/
State Accepted
Commit e56f7cd8c67bc94a7f5c3fd9ec3bf4cc726a3103
Headers show

Comments

Mads Kiilerich - Feb. 26, 2013, 2:41 a.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1361846443 -3600
# Branch stable
# Node ID c92e62e6b93838c00c749e85e4eab243e155dd58
# Parent  27f081501f9e04320ac94654c61e766764ee159e
tests: don't rely on broken behaviour in test-largefiles-cache.t

The test relied on the bug that 'pull largefiles from branchheads' didn't pull
any largefiles from tip revision when it seemed like no largefiles had been
checked out before.
Mads Kiilerich - Feb. 26, 2013, 6:11 p.m.
On 02/26/2013 06:16 AM, Kevin Bullock wrote:
> On 25 Feb 2013, at 8:41 PM, Mads Kiilerich wrote:
>
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1361846443 -3600
>> # Branch stable
>> # Node ID c92e62e6b93838c00c749e85e4eab243e155dd58
>> # Parent  27f081501f9e04320ac94654c61e766764ee159e
>> tests: don't rely on broken behaviour in test-largefiles-cache.t
>>
>> The test relied on the bug that 'pull largefiles from branchheads' didn't pull
>> any largefiles from tip revision when it seemed like no largefiles had been
>> checked out before.
>>
>> diff --git a/tests/test-largefiles-cache.t b/tests/test-largefiles-cache.t
>> --- a/tests/test-largefiles-cache.t
>> +++ b/tests/test-largefiles-cache.t
>> @@ -16,6 +16,9 @@
>>    $ echo large > large
>>    $ hg add --large large
>>    $ hg commit -m 'add largefile'
>> +  $ hg rm large
>> +  $ hg commit -m 'branchhead without largefile'
>> +  $ hg up -qr 0
>>    $ cd ..
>>
>> Discard all cached largefiles in USERCACHE
>> @@ -24,7 +27,7 @@
>>
>> Create mirror repo, and pull from source without largefile:
>> "pull" is used instead of "clone" for suppression of (1) updating to
>> -tip (= cahcing largefile from source repo), and (2) recording source
>> +tip (= caching largefile from source repo), and (2) recording source
> Looks like an unrelated spelling fix?

Absolutely correct. In the descriptive part of a test.

The guidelines requiring no unrelated changes are good and do serve a 
purpose. But sometimes it serve no purpose to follow them literally. 
They are an attempt of formalizing and generalizing the intention.

In this case I was doing trivial cleanup of the test file anyway. There 
is no way this could make a review harder or hide bugs or that anybody 
would care to apply or bisect one without the other. Fixing a typo in 
the passing in such a patch adds less overhead to the whole process than 
doing it in a separate patch. All time spent handling and discussing 
such a minor nice-to-have is a waste of time.

Please, let's get the proportions right and focus on stuff that matters.

/Mads
Matt Mackall - Feb. 26, 2013, 7:30 p.m.
On Tue, 2013-02-26 at 19:11 +0100, Mads Kiilerich wrote:
> On 02/26/2013 06:16 AM, Kevin Bullock wrote:
> > On 25 Feb 2013, at 8:41 PM, Mads Kiilerich wrote:
> >
> >> # HG changeset patch
> >> # User Mads Kiilerich <madski@unity3d.com>
> >> # Date 1361846443 -3600
> >> # Branch stable
> >> # Node ID c92e62e6b93838c00c749e85e4eab243e155dd58
> >> # Parent  27f081501f9e04320ac94654c61e766764ee159e
> >> tests: don't rely on broken behaviour in test-largefiles-cache.t
> >>
> >> The test relied on the bug that 'pull largefiles from branchheads' didn't pull
> >> any largefiles from tip revision when it seemed like no largefiles had been
> >> checked out before.
> >>
> >> diff --git a/tests/test-largefiles-cache.t b/tests/test-largefiles-cache.t
> >> --- a/tests/test-largefiles-cache.t
> >> +++ b/tests/test-largefiles-cache.t
> >> @@ -16,6 +16,9 @@
> >>    $ echo large > large
> >>    $ hg add --large large
> >>    $ hg commit -m 'add largefile'
> >> +  $ hg rm large
> >> +  $ hg commit -m 'branchhead without largefile'
> >> +  $ hg up -qr 0
> >>    $ cd ..
> >>
> >> Discard all cached largefiles in USERCACHE
> >> @@ -24,7 +27,7 @@
> >>
> >> Create mirror repo, and pull from source without largefile:
> >> "pull" is used instead of "clone" for suppression of (1) updating to
> >> -tip (= cahcing largefile from source repo), and (2) recording source
> >> +tip (= caching largefile from source repo), and (2) recording source
> > Looks like an unrelated spelling fix?
> 
> Absolutely correct. In the descriptive part of a test.
> 
> The guidelines requiring no unrelated changes are good and do serve a 
> purpose. But sometimes it serve no purpose to follow them literally. 

Breaking the rules is a false efficiency as the probability of
triggering an expensive rules discussion and/or resend is high. 
Further, the probability of triggering the expense does not drop in
proportion to the value of the unrelated change, so the expected
cost/benefit ratio goes to infinity as the value of the unrelated change
goes to zero.

So please do not break the rules, especially for nearly-zero-value
changes like typos in test descriptions. Better to not make such changes
at all than to spend an hour discussing them.

Patch

diff --git a/tests/test-largefiles-cache.t b/tests/test-largefiles-cache.t
--- a/tests/test-largefiles-cache.t
+++ b/tests/test-largefiles-cache.t
@@ -16,6 +16,9 @@ 
   $ echo large > large
   $ hg add --large large
   $ hg commit -m 'add largefile'
+  $ hg rm large
+  $ hg commit -m 'branchhead without largefile'
+  $ hg up -qr 0
   $ cd ..
 
 Discard all cached largefiles in USERCACHE
@@ -24,7 +27,7 @@ 
 
 Create mirror repo, and pull from source without largefile:
 "pull" is used instead of "clone" for suppression of (1) updating to
-tip (= cahcing largefile from source repo), and (2) recording source
+tip (= caching largefile from source repo), and (2) recording source
 repo as "default" path in .hg/hgrc.
 
   $ hg init mirror
@@ -35,7 +38,7 @@ 
   adding changesets
   adding manifests
   adding file changes
-  added 1 changesets with 1 changes to 1 files
+  added 2 changesets with 1 changes to 1 files
   (run 'hg update' to get a working copy)
   caching new largefiles
   0 largefiles cached
@@ -44,7 +47,7 @@ 
 but there is no cache file for it.  So, hg must treat it as
 "missing"(!) file.
 
-  $ hg update
+  $ hg update -r0
   getting changed largefiles
   error getting id 7f7097b041ccf68cc5561e9600da4655d21c6d18 from url file:$TESTTMP/mirror for file large: can't get file locally (glob)
   0 largefiles updated, 0 removed
@@ -61,7 +64,7 @@ 
 
 Update working directory to tip, again.
 
-  $ hg update
+  $ hg update -r0
   getting changed largefiles
   error getting id 7f7097b041ccf68cc5561e9600da4655d21c6d18 from url file:$TESTTMP/mirror for file large: can't get file locally (glob)
   0 largefiles updated, 0 removed
@@ -90,6 +93,7 @@ 
   $ chmod 660 large
   $ echo change >> large
   $ hg commit -m change
+  created new head
   $ ../ls-l.py .hg/largefiles/e151b474069de4ca6898f67ce2f2a7263adf8fea
   640