git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Andrei Rybak <rybak.a.v@gmail.com>,
	Elijah Newren <newren@gmail.com>,
	Elijah Newren <newren@gmail.com>,
	Elijah Newren <newren@gmail.com>
Subject: [PATCH v2] t6426: fix TODO about making test more comprehensive
Date: Sat, 14 Jan 2023 18:49:46 +0000	[thread overview]
Message-ID: <pull.1462.v2.git.1673722187025.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1462.git.1673584084761.gitgitgadget@gmail.com>

From: Elijah Newren <newren@gmail.com>

t6426.7 (a rename/add testcase) long had a TODO/FIXME comment about
how the test could be improved (with some commented out sample code
that had a few small errors), but those improvements were blocked on
other changes still in progress.  The necessary changes were put in
place years ago but the comment was forgotten.  Remove and fix the
commented out code section and finally remove the big TODO/FIXME
comment.

Signed-off-by: Elijah Newren <newren@gmail.com>
---
    t6426: fix TODO about making test more comprehensive
    
    See
    https://lore.kernel.org/git/CABPp-BFxK7SGs3wsOfozSw_Uvr-ynr+x8ciPV2Rmfx6Nr4si6g@mail.gmail.com/
    
    Changes since v1:
    
     * Removed spurious line, rearranged some lines so that checks on file
       "b" come before file "c".

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1462%2Fnewren%2Ft6426-fix-todo-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1462/newren/t6426-fix-todo-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1462

Range-diff vs v1:

 1:  6de47daaeeb ! 1:  68fd28e2547 t6426: fix TODO about making test more comprehensive
     @@ Commit message
      
       ## t/t6426-merge-skip-unneeded-updates.sh ##
      @@ t/t6426-merge-skip-unneeded-updates.sh: test_expect_success '2c: Modify b & add c VS rename b->c' '
     + 		test_i18ngrep "CONFLICT (.*/add):" out &&
     + 		test_must_be_empty err &&
       
     - 		# Make sure c WAS updated
     +-		# Make sure c WAS updated
     ++		git ls-files -s >index_files &&
     ++		test_line_count = 2 index_files &&
     ++
     ++		# Ensure b was removed
     ++		test_path_is_missing b &&
     ++
     ++		# Make sure c WAS updated...
       		test-tool chmtime --get c >new-mtime &&
      -		test $(cat old-mtime) -lt $(cat new-mtime)
      -
     @@ t/t6426-merge-skip-unneeded-updates.sh: test_expect_success '2c: Modify b & add
      -		#test_path_is_missing b
      +		test $(cat old-mtime) -lt $(cat new-mtime) &&
      +
     -+		git ls-files -s >index_files &&
     -+		test_line_count = 2 index_files &&
     -+
     ++		# ...and has correct index entries and working tree contents
      +		git rev-parse >actual :2:c :3:c &&
      +		git rev-parse >expect A:c  A:b  &&
      +		test_cmp expect actual &&
     @@ t/t6426-merge-skip-unneeded-updates.sh: test_expect_success '2c: Modify b & add
      +			-L "" \
      +			-L "B^0" \
      +			merged empty merge-me &&
     -+		sed -e "s/^\([<=>]\)/\1\1\1/" merged >merged-internal &&
     -+
     -+		test_cmp merged c &&
     -+
     -+		test_path_is_missing b
     ++		test_cmp merged c
       	)
       '
       


 t/t6426-merge-skip-unneeded-updates.sh | 58 ++++++++++----------------
 1 file changed, 23 insertions(+), 35 deletions(-)

diff --git a/t/t6426-merge-skip-unneeded-updates.sh b/t/t6426-merge-skip-unneeded-updates.sh
index 2bb8e7f09bb..fd21c1a4863 100755
--- a/t/t6426-merge-skip-unneeded-updates.sh
+++ b/t/t6426-merge-skip-unneeded-updates.sh
@@ -378,42 +378,30 @@ test_expect_success '2c: Modify b & add c VS rename b->c' '
 		test_i18ngrep "CONFLICT (.*/add):" out &&
 		test_must_be_empty err &&
 
-		# Make sure c WAS updated
+		git ls-files -s >index_files &&
+		test_line_count = 2 index_files &&
+
+		# Ensure b was removed
+		test_path_is_missing b &&
+
+		# Make sure c WAS updated...
 		test-tool chmtime --get c >new-mtime &&
-		test $(cat old-mtime) -lt $(cat new-mtime)
-
-		# FIXME: rename/add conflicts are horribly broken right now;
-		# when I get back to my patch series fixing it and
-		# rename/rename(2to1) conflicts to bring them in line with
-		# how add/add conflicts behave, then checks like the below
-		# could be added.  But that patch series is waiting until
-		# the rename-directory-detection series lands, which this
-		# is part of.  And in the mean time, I do not want to further
-		# enforce broken behavior.  So for now, the main test is the
-		# one above that err is an empty file.
-
-		#git ls-files -s >index_files &&
-		#test_line_count = 2 index_files &&
-
-		#git rev-parse >actual :2:c :3:c &&
-		#git rev-parse >expect A:b  A:c  &&
-		#test_cmp expect actual &&
-
-		#git cat-file -p A:b >>merged &&
-		#git cat-file -p A:c >>merge-me &&
-		#>empty &&
-		#test_must_fail git merge-file \
-		#	-L "Temporary merge branch 1" \
-		#	-L "" \
-		#	-L "Temporary merge branch 2" \
-		#	merged empty merge-me &&
-		#sed -e "s/^\([<=>]\)/\1\1\1/" merged >merged-internal &&
-
-		#git hash-object c               >actual &&
-		#git hash-object merged-internal >expect &&
-		#test_cmp expect actual &&
-
-		#test_path_is_missing b
+		test $(cat old-mtime) -lt $(cat new-mtime) &&
+
+		# ...and has correct index entries and working tree contents
+		git rev-parse >actual :2:c :3:c &&
+		git rev-parse >expect A:c  A:b  &&
+		test_cmp expect actual &&
+
+		git cat-file -p A:b >>merge-me &&
+		git cat-file -p A:c >>merged &&
+		>empty &&
+		test_must_fail git merge-file \
+			-L "HEAD" \
+			-L "" \
+			-L "B^0" \
+			merged empty merge-me &&
+		test_cmp merged c
 	)
 '
 

base-commit: 2b4f5a4e4bb102ac8d967cea653ed753b608193c
-- 
gitgitgadget

      parent reply	other threads:[~2023-01-14 18:49 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-13  4:28 [PATCH] t6426: fix TODO about making test more comprehensive Elijah Newren via GitGitGadget
2023-01-13 22:09 ` Andrei Rybak
2023-01-14  4:06   ` Elijah Newren
2023-01-14 18:49 ` Elijah Newren via GitGitGadget [this message]

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=pull.1462.v2.git.1673722187025.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=rybak.a.v@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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).