Re: [PATCH 10/30] directory rename detection: more involved edge/corner testcases
- From
Stefan Beller <sbeller@google.com>
- Date
- Nov 14, 2017, 00:42 UTC
- Message-ID
- <CAGZ79kaDWxx2g2OMY-VKJZD0-0t7V33644aqPLHq5Hppo_Be+w@mail.gmail.com>
- In-Reply-To
- <20171110190550.27059-11-newren@gmail.com>
On Fri, Nov 10, 2017 at 11:05 AM, Elijah Newren <newren@gmail.com> wrote:
Show 5 quoted lines
> +# In my opinion, testcases that are difficult to understand from this > +# section is due to difficulty in the testcase rather than the directory > +# renaming (similar to how t6042 and t6036 have difficult resolutions due > +# to the problem setup itself being complex). And I don't think the > +# error messages are a problem.
"In my opinion" ... sounds like commit message?
> +# On the other hand, the testcases in section 8 worry me slightly more...
The aforementioned section 8... :)
Show 6 quoted lines
> +# Testcase 7a, rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file
> +# Commit A: z/{b,c}
> +# Commit B: y/{b,c}
> +# Commit C: w/b, x/c, z/d
> +# Expected: y/d, CONFLICT(rename/rename for both z/b and z/c)
> +# NOTE: There's a rename of z/ here, y/ has more renames, so z/d -> y/d.But the creator of C intended to have z/d, not {w,x}/d, and as {w,x} == y, I am not sure I like this result. (I have no concrete counter example, just messy logic)
Show 6 quoted lines
> +# Testcase 7b, rename/rename(2to1), but only due to transitive rename
> +# (Related to testcase 1d)
> +# Commit A: z/{b,c}, x/d_1, w/d_2
> +# Commit B: y/{b,c,d_2}, x/d_1
> +# Commit C: z/{b,c,d_1}, w/d_2
> +# Expected: y/{b,c}, CONFLICT(rename/rename(2to1): x/d_1, w/d_2 -> y_d)Makes sense.
Show 6 quoted lines
> +# Testcase 7c, rename/rename(1to...2or3); transitive rename may add complexity
> +# (Related to testcases 3b and 5c)
> +# Commit A: z/{b,c}, x/d
> +# Commit B: y/{b,c}, w/d
> +# Commit C: z/{b,c,d}
> +# Expected: y/{b,c}, CONFLICT(x/d -> w/d vs. y/d)CONFLICT(x/d -> y/d vs w/d) ?
> +# NOTE: z/ was renamed to y/ so we do not want to report > +# either CONFLICT(x/d -> w/d vs. z/d) > +# or CONFLiCT x/d -> w/d vs. y/d vs. z/d)
"neither ... nor" instead of "not either or"?
Show 7 quoted lines
> +# Testcase 7d, transitive rename involved in rename/delete; how is it reported?
> +# (Related somewhat to testcases 5b and 8d)
> +# Commit A: z/{b,c}, x/d
> +# Commit B: y/{b,c}
> +# Commit C: z/{b,c,d}
> +# Expected: y/{b,c}, CONFLICT(delete x/d vs rename to y/d)
> +# NOTE: z->y so NOT CONFLICT(delete x/d vs rename to z/d)Show 16 quoted lines
> +# Testcase 7e, transitive rename in rename/delete AND dirs in the way
> +# (Very similar to 'both rename source and destination involved in D/F conflict' from t6022-merge-rename.sh)
> +# (Also related to testcases 9c and 9d)
> +# Commit A: z/{b,c}, x/d_1
> +# Commit B: y/{b,c,d/g}, x/d/f
> +# Commit C: z/{b,c,d_1}
> +# Expected: rename/delete(x/d_1->y/d_1 vs. None) + D/F conflict on y/d
> +# y/{b,c,d/g}, y/d_1~C^0, x/d/f
> +# NOTE: x/d/f may be slightly confusing here. x/d_1 -> z/d_1 implies
> +# there is a directory rename from x/ -> z/, performed by commit C.
> +# However, on the side of commit B, it renamed z/ -> y/, thus
> +# making a rename from x/ -> z/ when it was getting rid of z/ seems
> +# non-sensical. Further, putting x/d/f into y/d/f also doesn't
> +# make a lot of sense because commit B did the renaming of z to y
> +# and it created x/d/f, and it clearly made these things separate,
> +# so it doesn't make much sense to push these together.This is confusing.