git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [GSoC][PATCH 0/3] Use helper functions in test script
@ 2019-03-03 12:28 Rohit Ashiwal
  2019-03-03 12:28 ` [PATCH 1/3] test functions: Add new function `test_file_not_empty` Rohit Ashiwal
                   ` (4 more replies)
  0 siblings, 5 replies; 42+ messages in thread
From: Rohit Ashiwal @ 2019-03-03 12:28 UTC (permalink / raw)
  To: git
  Cc: Johannes.Schindelin, gitster, t.gummerer, christian.couder,
	Rohit Ashiwal

This patch ultimately aims to replace `test -(d|f|e|s)` calls in t3600-rm.sh
Previously we were using these to verify the presence of diretory/file, but
we already have helper functions, viz, `test_path_is_dir`, `test_path_is_file`,
`test_path_is_missing` and `test_file_not_empty` with better functionality

Helper functions are better as they provide better error messages and
improve readability. They are friendly to someone new to code.

Note: `test_file_not_empty` is implemented in [PATCH 1/3] of this mail

Rohit Ashiwal (3):
  test functions: Add new function `test_file_not_empty`
  t3600: refactor code according to contemporary guidelines
  t3600: use helper functions from test-lib-functions

 t/t3600-rm.sh           | 281 +++++++++++++++++++++-------------------
 t/test-lib-functions.sh |  10 ++
 2 files changed, 157 insertions(+), 134 deletions(-)

-- 
Thanks
Rohit


^ permalink raw reply	[flat|nested] 42+ messages in thread
* (no subject)
@ 2023-10-16 18:43 Dorcas Litunya
  2023-10-17 20:21 ` none Junio C Hamano
  0 siblings, 1 reply; 42+ messages in thread
From: Dorcas Litunya @ 2023-10-16 18:43 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: christian.couder, git

Bcc: 
Subject: Re: [PATCH] t/t7601: Modernize test scripts using functions
Reply-To: 
In-Reply-To: <xmqq1qdumrto.fsf@gitster.g>

On Mon, Oct 16, 2023 at 09:53:55AM -0700, Junio C Hamano wrote:
> Dorcas AnonoLitunya <anonolitunya@gmail.com> writes:
> 
> > Subject: Re: [PATCH] t/t7601: Modernize test scripts using functions
> 
> Let's try if we can pack a bit more information.  For example
> 
> Subject: [PATCH] t7601: use "test_path_is_file" etc. instead of "test -f"
> 
> would clarify what kind of modernization is done by this patch.
> 
> > The test script is currently using the command format 'test -f' to
> > check for existence or absence of files.
> 
> "is currently using" -> "uses".
> 
> > Replace it with new helper functions following the format
> > 'test_path_is_file'.
> 
> I am not sure what role "the format" plays in this picture.
> test_path_is_file is not new---it has been around for quite a while.
> 
> > Consequently, the patch also replaces the inverse command '! test -f' or
> > 'test ! -f' with new helper function following the format
> > 'test_path_is_missing'
> 
> A bit more on this later.
>
So should I replace this in the next version or leave this as is?
> > This adjustment using helper functions makes the code more readable and
> > easier to understand.
> 
> Looking good.  If I were writing this, I'll make the whole thing
> more like this, though:
> 
>     t7601: use "test_path_is_file" etc. instead of "test -f"
> 
>     Some tests in t7601 use "test -f" and "test ! -f" to see if a
>     path exists or is missing.  Use test_path_is_file and
>     test_path_is_missing helper functions to clarify these tests a
>     bit better.  This especially matters for the "missing" case,
>     because "test ! -f F" will be happy if "F" exists as a
>     directory, but the intent of the test is that "F" should not
>     exist, even as a directory.
> 
> 
> > diff --git a/t/t7601-merge-pull-config.sh b/t/t7601-merge-pull-config.sh
> > index bd238d89b0..e08767df66 100755
> > --- a/t/t7601-merge-pull-config.sh
> > +++ b/t/t7601-merge-pull-config.sh
> > @@ -349,13 +349,13 @@ test_expect_success 'Cannot rebase with multiple heads' '
> >  
> >  test_expect_success 'merge c1 with c2' '
> >  	git reset --hard c1 &&
> > -	test -f c0.c &&
> > -	test -f c1.c &&
> > -	test ! -f c2.c &&
> > -	test ! -f c3.c &&
> > +	test_path_is_file c0.c &&
> > +	test_path_is_file c1.c &&
> > +	test_path_is_missing c2.c &&
> > +	test_path_is_missing c3.c &&
> 
> The original says "We are happy if c2.c is not a file", so it would
> have been happy if by some mistake "git reset" created a directory
> there.  But the _intent_ of the test is that we do not have anything
> at c2.c, and the updated code expresses it better.

^ permalink raw reply	[flat|nested] 42+ messages in thread
* (no subject)
@ 2019-11-20  3:49 Han-Wen Nienhuys
  2019-11-20  4:52 ` none Junio C Hamano
  0 siblings, 1 reply; 42+ messages in thread
From: Han-Wen Nienhuys @ 2019-11-20  3:49 UTC (permalink / raw)
  To: git, Christian Couder, Johannes Schindelin

Hey folks,

I spent the last few weeks cobbling together an implementation of the
reftable format in C and in Go. I thought this would be cool to add to
git-core, but I doubt whether I will have enough time to see such an
effort through. Maybe some of you would want to try integrating it
into the Git-core code base?  Example code is here:

  https://github.com/google/reftable/blob/master/c/api.h#L153

cheers!
--
Han-Wen Nienhuys - Google Munich
I work 80%. Don't expect answers from me on Fridays.
--
Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

^ permalink raw reply	[flat|nested] 42+ messages in thread
* Re: [PATCH] doc: fix a typo and clarify a sentence
@ 2018-10-05  6:20 Junio C Hamano
  2018-10-10 15:20 ` Mihir Mehta
  0 siblings, 1 reply; 42+ messages in thread
From: Junio C Hamano @ 2018-10-05  6:20 UTC (permalink / raw)
  To: Mihir Mehta; +Cc: git, sunshine

Mihir Mehta <mihir@cs.utexas.edu> writes:

> diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
> index b180f1fa5..6173f569e 100644
> --- a/Documentation/git-diff.txt
> +++ b/Documentation/git-diff.txt
> @@ -72,8 +72,9 @@ two blob objects, or changes between two files on disk.
>  	This form is to view the changes on the branch containing
>  	and up to the second <commit>, starting at a common ancestor
>  	of both <commit>.  "git diff A\...B" is equivalent to
> -	"git diff $(git-merge-base A B) B".  You can omit any one
> -	of <commit>, which has the same effect as using HEAD instead.
> +	"git diff $(git merge-base A B) B".  You can omit any one

"git merge-base" is a more modern way to spell "git-merge-base" and
we have been trying to update the mention of the latter in the docs
to the former.  Thanks for doing this.

> +	of the two instances of <commit>, which has the same effect as

The paragraph is about <commit>...<commit> three-dot notation.  I
suspect that you wanted to say <commit>... and ...<commit> are
allowed, implying that a bare ... is not allowed and does not mean
the same thing as what HEAD...HEAD means.  But that is not the case.
Asking "git diff HEAD...HEAD" by omitting both may not give very
interesting output (it always becomes a no-op), but nevertheless it
is a valid thing to ask (iow "git diff $commit1...$commit2" is what
you can safely write without having to worry about one or both going
empty string).  So I'd rather not to see this change in this form.
It is an incomplete attempt to discourage use of <empty>...<empty>
but without giving enough justification.

	Side note.  I am not recommending to do so, but
	"discouragement with enough justification" would look like
	this.

	You can omit <commit> on any side of the three dots, which
	has the same effect as using HEAD instead.  Omitting both
	and leaving only three dots is not an error but that merely
	specifies a set of commits that are and are not reachable
	from HEAD at the same time, which by definition is an empty
	set, hence it is not very useful.

> +	using HEAD in its place.

> +++ b/Documentation/howto/update-hook-example.txt
> @@ -80,7 +80,7 @@ case "$1" in
>        info "The branch '$1' is new..."
>      else
>        # updating -- make sure it is a fast-forward
> -      mb=$(git-merge-base "$2" "$3")
> +      mb=$(git merge-base "$2" "$3")

I strongly suspect that inside update-hook, the original still
should work (iow, $GIT_EXEC_PATH should already have been prepended
to $PATH before a hoook is called).  But the updated form should
also work, and it is the form we humans need to type, so let's take
this change.

Thanks.

>        case "$mb,$2" in
>          "$2,$mb") info "Update is fast-forward" ;;
>  	*)	  noff=y; info "This is not a fast-forward update.";;

^ permalink raw reply	[flat|nested] 42+ messages in thread
* (unknown), 
@ 2016-04-11 19:04 miwilliams
  2016-04-11 19:18 ` none Matthieu Moy
  0 siblings, 1 reply; 42+ messages in thread
From: miwilliams @ 2016-04-11 19:04 UTC (permalink / raw)
  To: git

 From 7201fe08ede76e502211a781250c9a0b702a78b2 Mon Sep 17 00:00:00 2001
From: Mike Williams <miwilliams@google.com>
Date: Mon, 11 Apr 2016 14:18:39 -0400
Subject: [PATCH 1/1] wt-status: Remove '!!' from  
wt_status_collect_changed_cb

The wt_status_collect_changed_cb function uses an extraneous double  
negation (!!)
when determining whether or not a submodule has new commits.

Signed-off-by: Mike Williams <miwilliams@google.com>
---
  wt-status.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/wt-status.c b/wt-status.c
index ef74864..b955179 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -431,7 +431,7 @@ static void wt_status_collect_changed_cb(struct  
diff_queue_struct *q,
  			d->worktree_status = p->status;
  		d->dirty_submodule = p->two->dirty_submodule;
  		if (S_ISGITLINK(p->two->mode))
-			d->new_submodule_commits = !!hashcmp(p->one->sha1, p->two->sha1);
+			d->new_submodule_commits = hashcmp(p->one->sha1, p->two->sha1);
  	}
  }

-- 
2.8.0

^ permalink raw reply related	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2023-10-17 20:22 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-03 12:28 [GSoC][PATCH 0/3] Use helper functions in test script Rohit Ashiwal
2019-03-03 12:28 ` [PATCH 1/3] test functions: Add new function `test_file_not_empty` Rohit Ashiwal
2019-03-03 13:20   ` Junio C Hamano
2019-03-03 13:29     ` Rohit Ashiwal
2019-03-03 13:33       ` none Junio C Hamano
2019-03-03 14:07         ` Clearing logic Rohit Ashiwal
2019-03-03 16:19           ` Thomas Gummerer
2019-03-03 12:28 ` [PATCH 2/3] t3600: refactor code according to contemporary guidelines Rohit Ashiwal
2019-03-03 13:30   ` Junio C Hamano
2019-03-03 14:13     ` t3600: refactor code according to comtemporary guidelines Rohit Ashiwal
2019-03-03 12:28 ` [PATCH 3/3] t3600: use helper functions from test-lib-functions Rohit Ashiwal
2019-03-03 13:32   ` Junio C Hamano
2019-03-03 23:37 ` [GSoC][PATCH v2 0/3] Use helper functions in test script Rohit Ashiwal
2019-03-03 23:37   ` [GSoC][PATCH v2 1/3] test functions: add function `test_file_not_empty` Rohit Ashiwal
2019-03-04  3:45     ` Junio C Hamano
2019-03-03 23:37   ` [GSoC][PATCH v2 2/3] t3600: restructure code according to contemporary guidelines Rohit Ashiwal
2019-03-04  4:17     ` Junio C Hamano
2019-03-03 23:37   ` [GSoC][PATCH v2 3/3] t3600: use helpers to replace test -d/f/e/s <path> Rohit Ashiwal
2019-03-04 12:07 ` [GSoC][PATCH v3 0/3] Use helper functions in test script Rohit Ashiwal
2019-03-04 12:07   ` [GSoC][PATCH v3 1/3] test functions: add function `test_file_not_empty` Rohit Ashiwal
2019-03-05  0:17     ` Eric Sunshine
2019-03-05 12:43       ` Junio C Hamano
2019-03-05 13:27       ` [GSoc][PATCH " Rohit Ashiwal
2019-03-04 12:08   ` [GSoC][PATCH v3 2/3] t3600: modernize style Rohit Ashiwal
2019-03-05  0:36     ` Eric Sunshine
2019-03-05 12:44       ` Junio C Hamano
2019-03-04 12:08   ` [GSoC][PATCH v3 3/3] t3600: use helpers to replace test -d/f/e/s <path> Rohit Ashiwal
2019-03-05  0:42     ` Eric Sunshine
2019-03-05 13:42       ` Rohit Ashiwal
2019-03-05 14:03         ` Eric Sunshine
2019-03-05 14:21           ` [GSoC][PATCH v2 " Rohit Ashiwal
2019-03-05 14:57             ` Eric Sunshine
2019-03-05 23:38               ` Rohit Ashiwal
2019-03-08  5:38               ` [GSoC][PATCH v2 3/3] t3600: use helpers to replace test -d/f/e/s <path> Junio C Hamano
2019-03-08  9:51                 ` Eric Sunshine
2019-03-11  1:54                   ` Junio C Hamano
2019-03-05  0:09   ` [GSoC][PATCH v3 0/3] Use helper functions in test script Eric Sunshine
  -- strict thread matches above, loose matches on Subject: below --
2023-10-16 18:43 Dorcas Litunya
2023-10-17 20:21 ` none Junio C Hamano
2019-11-20  3:49 Han-Wen Nienhuys
2019-11-20  4:52 ` none Junio C Hamano
2019-11-20  5:00   ` none Han-Wen Nienhuys
2018-10-05  6:20 [PATCH] doc: fix a typo and clarify a sentence Junio C Hamano
2018-10-10 15:20 ` Mihir Mehta
2018-10-10 22:19   ` none Junio C Hamano
2016-04-11 19:04 (unknown), miwilliams
2016-04-11 19:18 ` none Matthieu Moy

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 NNTP newsgroup(s).