git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Eric Sunshine <sunshine@sunshineco.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Elijah Newren <newren@gmail.com>,
	Fabian Stelzer <fs@gigacodes.de>,
	Eric Sunshine <sunshine@sunshineco.com>
Subject: [PATCH 03/15] t/chainlint/*.test: generalize self-test commentary
Date: Mon, 13 Dec 2021 01:30:47 -0500	[thread overview]
Message-ID: <20211213063059.19424-4-sunshine@sunshineco.com> (raw)
In-Reply-To: <20211213063059.19424-1-sunshine@sunshineco.com>

The purpose of chainlint.sed is to detect &&-chain breakage only within
subshells (one level deep); it doesn't bother checking for top-level
&&-chain breakage since the &&-chain checker built into t/test-lib.sh
should detect broken &&-chains outside of subshells by making them
magically exit with code 117. However, this division of labor may not
always be the case if a more capable chainlint implementation is ever
developed. Beyond that, due to being sed-based and due to its use of
heuristics, chainlint.sed has several limitations (such as being unable
to detect &&-chain breakage in subshells more than one level deep since
it only manually emulates recursion into a subshell).

Some of the comments in the chainlint self-tests unnecessarily reflect
the limitations of chainlint.sed even though those limitations are not
what is being tested. Therefore, simplify and generalize the comments to
explain only what is being tested, thus ensuring that they won't become
outdated if a more capable chainlint is ever developed.

Signed-off-by: Eric Sunshine <sunshine@sunshineco.com>
---
 t/chainlint/blank-line.test      | 2 +-
 t/chainlint/block.test           | 3 +--
 t/chainlint/cuddled.test         | 3 +--
 t/chainlint/nested-subshell.test | 1 -
 t/chainlint/one-liner.test       | 2 +-
 t/chainlint/semicolon.test       | 4 ++--
 6 files changed, 6 insertions(+), 9 deletions(-)

diff --git a/t/chainlint/blank-line.test b/t/chainlint/blank-line.test
index f6dd14302b..0fdf15b3e1 100644
--- a/t/chainlint/blank-line.test
+++ b/t/chainlint/blank-line.test
@@ -3,7 +3,7 @@
 	nothing &&
 
 	something
-# LINT: swallow blank lines since final _statement_ before subshell end is
+# LINT: ignore blank lines since final _statement_ before subshell end is
 # LINT: significant to "&&"-check, not final _line_ (which might be blank)
 
 
diff --git a/t/chainlint/block.test b/t/chainlint/block.test
index d859151af1..0a82fd579f 100644
--- a/t/chainlint/block.test
+++ b/t/chainlint/block.test
@@ -1,6 +1,5 @@
 (
-# LINT: missing "&&" in block not currently detected (for consistency with
-# LINT: --chain-lint at top level and to provide escape hatch if needed)
+# LINT: missing "&&" after first "echo"
 	foo &&
 	{
 		echo a
diff --git a/t/chainlint/cuddled.test b/t/chainlint/cuddled.test
index 0499fa4180..257b5b5eed 100644
--- a/t/chainlint/cuddled.test
+++ b/t/chainlint/cuddled.test
@@ -1,5 +1,4 @@
-# LINT: first subshell statement cuddled with opening "("; for implementation
-# LINT: simplicity, "(..." is split into two lines, "(" and "..."
+# LINT: first subshell statement cuddled with opening "("
 (cd foo &&
 	bar
 ) &&
diff --git a/t/chainlint/nested-subshell.test b/t/chainlint/nested-subshell.test
index 998b05a47d..440ee9992d 100644
--- a/t/chainlint/nested-subshell.test
+++ b/t/chainlint/nested-subshell.test
@@ -7,7 +7,6 @@
 
 	cd foo &&
 	(
-# LINT: nested multi-line subshell not presently checked for missing "&&"
 		echo a
 		echo b
 	) >file
diff --git a/t/chainlint/one-liner.test b/t/chainlint/one-liner.test
index ec9acb9825..69796d7505 100644
--- a/t/chainlint/one-liner.test
+++ b/t/chainlint/one-liner.test
@@ -3,7 +3,7 @@
 (foo && bar) |
 (foo && bar) >baz &&
 
-# LINT: top-level one-liner subshell missing internal "&&"
+# LINT: top-level one-liner subshell missing internal "&&" and broken &&-chain
 (foo; bar) &&
 (foo; bar) |
 (foo; bar) >baz
diff --git a/t/chainlint/semicolon.test b/t/chainlint/semicolon.test
index d82c8ebbc0..67e1192c50 100644
--- a/t/chainlint/semicolon.test
+++ b/t/chainlint/semicolon.test
@@ -15,11 +15,11 @@
 	cat foo; echo bar
 ) &&
 (
-# LINT: unnecessary terminating semicolon
+# LINT: semicolon unnecessary but legitimate
 	foo;
 ) &&
 (cd foo &&
 	for i in a b c; do
-# LINT: unnecessary terminating semicolon
+# LINT: semicolon unnecessary but legitimate
 		echo;
 	done)
-- 
2.34.1.397.gfae76fe5da


  parent reply	other threads:[~2021-12-13  6:32 UTC|newest]

Thread overview: 34+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-13  6:30 [PATCH 00/15] generalize chainlint self-tests Eric Sunshine
2021-12-13  6:30 ` [PATCH 01/15] t/chainlint/*.test: don't use invalid shell syntax Eric Sunshine
2021-12-13  6:30 ` [PATCH 02/15] t/chainlint/*.test: fix invalid test cases due to mixing quote types Eric Sunshine
2021-12-13  6:30 ` Eric Sunshine [this message]
2021-12-13  6:30 ` [PATCH 04/15] t/chainlint/one-liner: avoid overly intimate chainlint.sed knowledge Eric Sunshine
2021-12-13  6:30 ` [PATCH 05/15] t/Makefile: optimize chainlint self-test Eric Sunshine
2021-12-13 10:09   ` [RFC PATCH] t/Makefile: use dependency graph for "check-chainlint" Ævar Arnfjörð Bjarmason
2021-12-14  7:44     ` Eric Sunshine
2021-12-14 12:34       ` Ævar Arnfjörð Bjarmason
2021-12-13 10:22   ` [PATCH 05/15] t/Makefile: optimize chainlint self-test Fabian Stelzer
2021-12-13 14:27     ` Eric Sunshine
2021-12-13 15:43       ` Fabian Stelzer
2021-12-13 16:02         ` Eric Sunshine
2021-12-13 16:11           ` Ævar Arnfjörð Bjarmason
2021-12-13 17:05             ` Eric Sunshine
2021-12-13 17:25               ` Eric Sunshine
2021-12-13 19:33                 ` Ævar Arnfjörð Bjarmason
2021-12-13 21:37                   ` Eric Sunshine
2021-12-13 16:14           ` Fabian Stelzer
2021-12-16 13:17         ` Ævar Arnfjörð Bjarmason
2021-12-16 15:47           ` Eric Sunshine
2021-12-16 19:26             ` Ævar Arnfjörð Bjarmason
2021-12-13  6:30 ` [PATCH 06/15] chainlint.sed: improve ?!AMP?! placement accuracy Eric Sunshine
2021-12-13  6:30 ` [PATCH 07/15] chainlint.sed: improve ?!SEMI?! " Eric Sunshine
2021-12-13  6:30 ` [PATCH 08/15] chainlint.sed: tolerate harmless ";" at end of last line in block Eric Sunshine
2021-12-13  6:30 ` [PATCH 09/15] chainlint.sed: drop unnecessary distinction between ?!AMP?! and ?!SEMI?! Eric Sunshine
2021-12-13  6:30 ` [PATCH 10/15] chainlint.sed: drop subshell-closing ">" annotation Eric Sunshine
2021-12-13  6:30 ` [PATCH 11/15] chainlint.sed: make here-doc "<<-" operator recognition more POSIX-like Eric Sunshine
2021-12-13  6:30 ` [PATCH 12/15] chainlint.sed: don't mistake `<< word` in string as here-doc operator Eric Sunshine
2021-12-13  6:30 ` [PATCH 13/15] chainlint.sed: stop throwing away here-doc tags Eric Sunshine
2021-12-13  6:30 ` [PATCH 14/15] chainlint.sed: swallow comments consistently Eric Sunshine
2021-12-13  6:30 ` [PATCH 15/15] chainlint.sed: stop splitting "(..." into separate lines "(" and "..." Eric Sunshine
2021-12-15  0:00 ` [PATCH 00/15] generalize chainlint self-tests Elijah Newren
2021-12-15  3:15   ` Eric Sunshine

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=20211213063059.19424-4-sunshine@sunshineco.com \
    --to=sunshine@sunshineco.com \
    --cc=fs@gigacodes.de \
    --cc=git@vger.kernel.org \
    --cc=newren@gmail.com \
    --cc=peff@peff.net \
    /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 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).