Skip to content

Commit fdd8f6c

Browse files
committed
rn-115: add article about using tee in shell tests
1 parent 82a2519 commit fdd8f6c

File tree

1 file changed

+50
-2
lines changed

1 file changed

+50
-2
lines changed

rev_news/drafts/edition-115.md

+50-2
Original file line numberDiff line numberDiff line change
@@ -76,9 +76,57 @@ This edition covers what happened during the months of August and September 2024
7676
See this [Outreachy webpage](https://www.outreachy.org/docs/applicant/),
7777
for more information about the application process for contributors.
7878

79-
<!---
8079
### Reviews
81-
-->
80+
81+
* [[PATCH] tests: drop use of 'tee' that hides exit status](https://lore.kernel.org/git/[email protected]/)
82+
83+
Junio Hamano, the Git maintainer, sent a patch removing uses of the
84+
`tee` command from test scripts.
85+
[tee(1)](https://en.wikipedia.org/wiki/Tee_(command)) is a shell
86+
command that duplicates its input to both its output and to one or
87+
more files. The issue was that in some test scripts it was used
88+
after a [pipe](https://en.wikipedia.org/wiki/Pipeline_(Unix)) to
89+
directly duplicate the output of a Git command, so using a format
90+
like:
91+
92+
`git COMMAND OPTIONS... | tee FILE...`
93+
94+
And it's not a good idea to use a pipe after a Git command because
95+
pipes discard the exit code of the command before them, so the exit
96+
code of the whole line is only the exit code of the command after
97+
the pipe, here `tee`. In Git tests though, we wouldn't want a test
98+
to pass if the Git command fails when it should succeed.
99+
100+
As there was no reason to hide the exit code of the Git commands in
101+
the tests that used `tee`, Junio's patch basically just replaced
102+
`| tee` with a simple `>`
103+
[redirection](https://en.wikipedia.org/wiki/Redirection_(computing)).
104+
105+
Rubén Justo replied to Junio suggesting a wording improvement in the
106+
commit message, but otherwise agreeing with the patch.
107+
108+
Johannes Schindelin, alias Dscho, also replied to Junio saying that
109+
having something that duplicates the output of the Git command to
110+
standard output could still be useful for debugging, especially when
111+
dealing with CI failures. He showed an implementation of a
112+
`redirect_and_show()` helper function that could perhaps be used
113+
instead of `tee`, but agreed that it might be overkill and hoped that
114+
having more unit tests written in C could help.
115+
116+
Jeff King, alias Peff, replied to Dscho saying that a better help
117+
for debugging CI failures might be to have a way to automatically
118+
save the state of the test directory, called
119+
`trash directory.tXXXX-YYYY` where `tXXXX-YYYY` is related to the
120+
name of the test script.
121+
122+
Junio also replied to Dscho saying that a simple `cat FILE`
123+
instruction could be added after the lines where `| tee` was
124+
replaced with a redirection to make sure the Git command output
125+
appeared on standard output. Junio also agreed with Peff that saving
126+
the state of test directories would be best to debug CI failures.
127+
128+
A version of Junio's patch taking into account the wording
129+
improvement suggested by Rubén was eventually merged.
82130

83131
<!---
84132
### Support

0 commit comments

Comments
 (0)