From: Elijah Newren <newren@gmail.com>
To: git@vger.kernel.org
Cc: sbeller@google.com, szeder.dev@gmail.com, j6t@kdbg.org,
jrnieder@gmail.com, peff@peff.net, git@matthieu-moy.fr,
Elijah Newren <newren@gmail.com>
Subject: [PATCHv6 07/31] directory rename detection: more involved edge/corner testcases
Date: Fri, 5 Jan 2018 12:26:47 -0800 [thread overview]
Message-ID: <20180105202711.24311-8-newren@gmail.com> (raw)
In-Reply-To: <20180105202711.24311-1-newren@gmail.com>
Signed-off-by: Elijah Newren <newren@gmail.com>
---
t/t6043-merge-rename-directories.sh | 396 ++++++++++++++++++++++++++++++++++++
1 file changed, 396 insertions(+)
diff --git a/t/t6043-merge-rename-directories.sh b/t/t6043-merge-rename-directories.sh
index dc3fc66e5..38ca791e9 100755
--- a/t/t6043-merge-rename-directories.sh
+++ b/t/t6043-merge-rename-directories.sh
@@ -1508,4 +1508,400 @@ test_expect_success '6e-check: Add/add from one side' '
# side of history is the one doing the renaming.
###########################################################################
+
+###########################################################################
+# SECTION 7: More involved Edge/Corner cases
+#
+# The ruleset we have generated in the above sections seems to provide
+# well-defined merges. But can we find edge/corner cases that either (a)
+# are harder for users to understand, or (b) have a resolution that is
+# non-intuitive or suboptimal?
+#
+# The testcases in this section dive into cases that I've tried to craft in
+# a way to find some that might be surprising to users or difficult for
+# them to understand (the next section will look at non-intuitive or
+# suboptimal merge results). Some of the testcases are similar to ones
+# from past sections, but have been simplified to try to highlight error
+# messages using a "modified" path (due to the directory rename). Are
+# users okay with these?
+#
+# 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.
+#
+# On the other hand, the testcases in section 8 worry me slightly more...
+###########################################################################
+
+# Testcase 7a, rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file
+# Commit O: z/{b,c}
+# Commit A: y/{b,c}
+# Commit B: 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.
+
+test_expect_success '7a-setup: rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file' '
+ test_create_repo 7a &&
+ (
+ cd 7a &&
+
+ mkdir z &&
+ echo b >z/b &&
+ echo c >z/c &&
+ git add z &&
+ test_tick &&
+ git commit -m "O" &&
+
+ git branch O &&
+ git branch A &&
+ git branch B &&
+
+ git checkout A &&
+ git mv z y &&
+ test_tick &&
+ git commit -m "A" &&
+
+ git checkout B &&
+ mkdir w &&
+ mkdir x &&
+ git mv z/b w/ &&
+ git mv z/c x/ &&
+ echo d > z/d &&
+ git add z/d &&
+ test_tick &&
+ git commit -m "B"
+ )
+'
+
+test_expect_failure '7a-check: rename-dir vs. rename-dir (NOT split evenly) PLUS add-other-file' '
+ (
+ cd 7a &&
+
+ git checkout A^0 &&
+
+ test_must_fail git merge -s recursive B^0 >out &&
+ test_i18ngrep "CONFLICT (rename/rename).*z/b.*y/b.*w/b" out &&
+ test_i18ngrep "CONFLICT (rename/rename).*z/c.*y/c.*x/c" out &&
+
+ git ls-files -s >out &&
+ test_line_count = 7 out &&
+ git ls-files -u >out &&
+ test_line_count = 6 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :1:z/b :2:y/b :3:w/b :1:z/c :2:y/c :3:x/c :0:y/d &&
+ git rev-parse >expect \
+ O:z/b O:z/b O:z/b O:z/c O:z/c O:z/c B:z/d &&
+ test_cmp expect actual &&
+
+ git hash-object >actual \
+ y/b w/b y/c x/c &&
+ git rev-parse >expect \
+ O:z/b O:z/b O:z/c O:z/c &&
+ test_cmp expect actual
+ )
+'
+
+# Testcase 7b, rename/rename(2to1), but only due to transitive rename
+# (Related to testcase 1d)
+# Commit O: z/{b,c}, x/d_1, w/d_2
+# Commit A: y/{b,c,d_2}, x/d_1
+# Commit B: z/{b,c,d_1}, w/d_2
+# Expected: y/{b,c}, CONFLICT(rename/rename(2to1): x/d_1, w/d_2 -> y_d)
+
+test_expect_success '7b-setup: rename/rename(2to1), but only due to transitive rename' '
+ test_create_repo 7b &&
+ (
+ cd 7b &&
+
+ mkdir z &&
+ mkdir x &&
+ mkdir w &&
+ echo b >z/b &&
+ echo c >z/c &&
+ echo d1 > x/d &&
+ echo d2 > w/d &&
+ git add z x w &&
+ test_tick &&
+ git commit -m "O" &&
+
+ git branch O &&
+ git branch A &&
+ git branch B &&
+
+ git checkout A &&
+ git mv z y &&
+ git mv w/d y/ &&
+ test_tick &&
+ git commit -m "A" &&
+
+ git checkout B &&
+ git mv x/d z/ &&
+ rmdir x &&
+ test_tick &&
+ git commit -m "B"
+ )
+'
+
+test_expect_failure '7b-check: rename/rename(2to1), but only due to transitive rename' '
+ (
+ cd 7b &&
+
+ git checkout A^0 &&
+
+ test_must_fail git merge -s recursive B^0 >out &&
+ test_i18ngrep "CONFLICT (rename/rename)" out &&
+
+ git ls-files -s >out &&
+ test_line_count = 4 out &&
+ git ls-files -u >out &&
+ test_line_count = 2 out &&
+ git ls-files -o >out &&
+ test_line_count = 3 out &&
+
+ git rev-parse >actual \
+ :0:y/b :0:y/c :2:y/d :3:y/d &&
+ git rev-parse >expect \
+ O:z/b O:z/c O:w/d O:x/d &&
+ test_cmp expect actual &&
+
+ test_path_is_missing y/d &&
+ test_path_is_file y/d~HEAD &&
+ test_path_is_file y/d~B^0 &&
+
+ git hash-object >actual \
+ y/d~HEAD y/d~B^0 &&
+ git rev-parse >expect \
+ O:w/d O:x/d &&
+ test_cmp expect actual
+ )
+'
+
+# Testcase 7c, rename/rename(1to...2or3); transitive rename may add complexity
+# (Related to testcases 3b and 5c)
+# Commit O: z/{b,c}, x/d
+# Commit A: y/{b,c}, w/d
+# Commit B: z/{b,c,d}
+# Expected: y/{b,c}, CONFLICT(x/d -> w/d vs. y/d)
+# NOTE: z/ was renamed to y/ so we do want to report
+# neither CONFLICT(x/d -> w/d vs. z/d)
+# nor CONFLiCT x/d -> w/d vs. y/d vs. z/d)
+
+test_expect_success '7c-setup: rename/rename(1to...2or3); transitive rename may add complexity' '
+ test_create_repo 7c &&
+ (
+ cd 7c &&
+
+ mkdir z &&
+ echo b >z/b &&
+ echo c >z/c &&
+ mkdir x &&
+ echo d >x/d &&
+ git add z x &&
+ test_tick &&
+ git commit -m "O" &&
+
+ git branch O &&
+ git branch A &&
+ git branch B &&
+
+ git checkout A &&
+ git mv z y &&
+ git mv x w &&
+ test_tick &&
+ git commit -m "A" &&
+
+ git checkout B &&
+ git mv x/d z/ &&
+ rmdir x &&
+ test_tick &&
+ git commit -m "B"
+ )
+'
+
+test_expect_failure '7c-check: rename/rename(1to...2or3); transitive rename may add complexity' '
+ (
+ cd 7c &&
+
+ git checkout A^0 &&
+
+ test_must_fail git merge -s recursive B^0 >out &&
+ test_i18ngrep "CONFLICT (rename/rename).*x/d.*w/d.*y/d" out &&
+
+ git ls-files -s >out &&
+ test_line_count = 5 out &&
+ git ls-files -u >out &&
+ test_line_count = 3 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :0:y/b :0:y/c :1:x/d :2:w/d :3:y/d &&
+ git rev-parse >expect \
+ O:z/b O:z/c O:x/d O:x/d O:x/d &&
+ test_cmp expect actual
+ )
+'
+
+# Testcase 7d, transitive rename involved in rename/delete; how is it reported?
+# (Related somewhat to testcases 5b and 8d)
+# Commit O: z/{b,c}, x/d
+# Commit A: y/{b,c}
+# Commit B: 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)
+
+test_expect_success '7d-setup: transitive rename involved in rename/delete; how is it reported?' '
+ test_create_repo 7d &&
+ (
+ cd 7d &&
+
+ mkdir z &&
+ echo b >z/b &&
+ echo c >z/c &&
+ mkdir x &&
+ echo d >x/d &&
+ git add z x &&
+ test_tick &&
+ git commit -m "O" &&
+
+ git branch O &&
+ git branch A &&
+ git branch B &&
+
+ git checkout A &&
+ git mv z y &&
+ git rm -rf x &&
+ test_tick &&
+ git commit -m "A" &&
+
+ git checkout B &&
+ git mv x/d z/ &&
+ rmdir x &&
+ test_tick &&
+ git commit -m "B"
+ )
+'
+
+test_expect_failure '7d-check: transitive rename involved in rename/delete; how is it reported?' '
+ (
+ cd 7d &&
+
+ git checkout A^0 &&
+
+ test_must_fail git merge -s recursive B^0 >out &&
+ test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out &&
+
+ git ls-files -s >out &&
+ test_line_count = 3 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 1 out &&
+
+ git rev-parse >actual \
+ :0:y/b :0:y/c :3:y/d &&
+ git rev-parse >expect \
+ O:z/b O:z/c O:x/d &&
+ test_cmp expect actual
+ )
+'
+
+# 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 O: z/{b,c}, x/d_1
+# Commit A: y/{b,c,d/g}, x/d/f
+# Commit B: 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~B^0, x/d/f
+
+# NOTE: The main path of interest here is d_1 and where it ends up, but
+# this is actually a case that has two potential directory renames
+# involved and D/F conflict(s), so it makes sense to walk through
+# each step.
+#
+# Commit A renames z/ -> y/. Thus everything that B adds to z/
+# should be instead moved to y/. This gives us the D/F conflict on
+# y/d because x/d_1 -> z/d_1 -> y/d_1 conflicts with y/d/g.
+#
+# Further, commit B renames x/ -> z/, thus everything A adds to x/
+# should instead be moved to z/...BUT we removed z/ and renamed it
+# to y/, so maybe everything should move not from x/ to z/, but
+# from x/ to z/ to y/. Doing so might make sense from the logic so
+# far, but note that commit A had both an x/ and a y/; it did the
+# renaming of z/ to y/ and created x/d/f and it clearly made these
+# things separate, so it doesn't make much sense to push these
+# together. Doing so is what I'd call a doubly transitive rename;
+# see testcases 9c and 9d for further discussion of this issue and
+# how it's resolved.
+
+test_expect_success '7e-setup: transitive rename in rename/delete AND dirs in the way' '
+ test_create_repo 7e &&
+ (
+ cd 7e &&
+
+ mkdir z &&
+ echo b >z/b &&
+ echo c >z/c &&
+ mkdir x &&
+ echo d1 >x/d &&
+ git add z x &&
+ test_tick &&
+ git commit -m "O" &&
+
+ git branch O &&
+ git branch A &&
+ git branch B &&
+
+ git checkout A &&
+ git mv z y &&
+ git rm x/d &&
+ mkdir -p x/d &&
+ mkdir -p y/d &&
+ echo f >x/d/f &&
+ echo g >y/d/g &&
+ git add x/d/f y/d/g &&
+ test_tick &&
+ git commit -m "A" &&
+
+ git checkout B &&
+ git mv x/d z/ &&
+ rmdir x &&
+ test_tick &&
+ git commit -m "B"
+ )
+'
+
+test_expect_failure '7e-check: transitive rename in rename/delete AND dirs in the way' '
+ (
+ cd 7e &&
+
+ git checkout A^0 &&
+
+ test_must_fail git merge -s recursive B^0 >out &&
+ test_i18ngrep "CONFLICT (rename/delete).*x/d.*y/d" out &&
+
+ git ls-files -s >out &&
+ test_line_count = 5 out &&
+ git ls-files -u >out &&
+ test_line_count = 1 out &&
+ git ls-files -o >out &&
+ test_line_count = 2 out &&
+
+ git rev-parse >actual \
+ :0:x/d/f :0:y/d/g :0:y/b :0:y/c :3:y/d &&
+ git rev-parse >expect \
+ A:x/d/f A:y/d/g O:z/b O:z/c O:x/d &&
+ test_cmp expect actual &&
+
+ git hash-object y/d~B^0 >actual &&
+ git rev-parse O:x/d >expect &&
+ test_cmp expect actual
+ )
+'
+
test_done
--
2.14.2
next prev parent reply other threads:[~2018-01-05 20:28 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-05 20:26 [PATCHv6 00/31] Add directory rename detection to git Elijah Newren
2018-01-05 20:26 ` [PATCHv6 01/31] directory rename detection: basic testcases Elijah Newren
2018-01-05 20:26 ` [PATCHv6 02/31] directory rename detection: directory splitting testcases Elijah Newren
2018-01-05 20:26 ` [PATCHv6 03/31] directory rename detection: testcases to avoid taking detection too far Elijah Newren
2018-01-26 11:37 ` SZEDER Gábor
2018-01-26 16:55 ` Elijah Newren
2018-01-05 20:26 ` [PATCHv6 04/31] directory rename detection: partially renamed directory testcase/discussion Elijah Newren
2018-01-05 20:26 ` [PATCHv6 05/31] directory rename detection: files/directories in the way of some renames Elijah Newren
2018-01-05 20:26 ` [PATCHv6 06/31] directory rename detection: testcases checking which side did the rename Elijah Newren
2018-01-05 20:26 ` Elijah Newren [this message]
2018-01-05 20:26 ` [PATCHv6 08/31] directory rename detection: testcases exploring possibly suboptimal merges Elijah Newren
2018-01-05 20:26 ` [PATCHv6 09/31] directory rename detection: miscellaneous testcases to complete coverage Elijah Newren
2018-01-05 20:26 ` [PATCHv6 10/31] directory rename detection: tests for handling overwriting untracked files Elijah Newren
2018-01-05 20:26 ` [PATCHv6 11/31] directory rename detection: tests for handling overwriting dirty files Elijah Newren
2018-01-05 20:26 ` [PATCHv6 12/31] merge-recursive: move the get_renames() function Elijah Newren
2018-01-05 20:26 ` [PATCHv6 13/31] merge-recursive: introduce new functions to handle rename logic Elijah Newren
2018-01-05 20:26 ` [PATCHv6 14/31] merge-recursive: fix leaks of allocated renames and diff_filepairs Elijah Newren
2018-01-05 20:26 ` [PATCHv6 15/31] merge-recursive: make !o->detect_rename codepath more obvious Elijah Newren
2018-01-05 20:26 ` [PATCHv6 16/31] merge-recursive: split out code for determining diff_filepairs Elijah Newren
2018-01-05 20:26 ` [PATCHv6 17/31] merge-recursive: add a new hashmap for storing directory renames Elijah Newren
2018-01-05 20:26 ` [PATCHv6 18/31] merge-recursive: make a helper function for cleanup for handle_renames Elijah Newren
2018-01-05 20:26 ` [PATCHv6 19/31] merge-recursive: add get_directory_renames() Elijah Newren
2018-01-05 20:27 ` [PATCHv6 20/31] merge-recursive: check for directory level conflicts Elijah Newren
2018-01-05 20:27 ` [PATCHv6 21/31] merge-recursive: add a new hashmap for storing file collisions Elijah Newren
2018-01-05 20:27 ` [PATCHv6 22/31] merge-recursive: add computation of collisions due to dir rename & merging Elijah Newren
2018-01-05 20:27 ` [PATCHv6 23/31] merge-recursive: check for file level conflicts then get new name Elijah Newren
2018-01-05 20:27 ` [PATCHv6 24/31] merge-recursive: when comparing files, don't include trees Elijah Newren
2018-01-05 20:27 ` [PATCHv6 25/31] merge-recursive: apply necessary modifications for directory renames Elijah Newren
2018-01-05 20:27 ` [PATCHv6 26/31] merge-recursive: avoid clobbering untracked files with " Elijah Newren
2018-01-05 20:27 ` [PATCHv6 27/31] merge-recursive: fix overwriting dirty files involved in renames Elijah Newren
2018-01-05 20:27 ` [PATCHv6 28/31] merge-recursive: fix remaining directory rename + dirty overwrite cases Elijah Newren
2018-01-05 20:27 ` [PATCHv6 29/31] directory rename detection: new testcases showcasing a pair of bugs Elijah Newren
2018-01-05 20:27 ` [PATCHv6 30/31] merge-recursive: avoid spurious rename/rename conflict from dir renames Elijah Newren
2018-01-05 20:27 ` [PATCHv6 31/31] merge-recursive: ensure we write updates for directory-renamed file Elijah Newren
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180105202711.24311-8-newren@gmail.com \
--to=newren@gmail.com \
--cc=git@matthieu-moy.fr \
--cc=git@vger.kernel.org \
--cc=j6t@kdbg.org \
--cc=jrnieder@gmail.com \
--cc=peff@peff.net \
--cc=sbeller@google.com \
--cc=szeder.dev@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.