Re: [PATCH v2 6/6] t9109-git-svn-props.sh: split up several pipes
From: Matthew DeVore <hidden>
Date: 2018-09-19 18:43:20
On Wed, Sep 19, 2018 at 5:50 AM Eric Sunshine [off-list ref] wrote:
If so, then please use test_cmp() rather than raw 'cmp' since test_cmp() will show the actual difference between the expected and actual files, which can be helpful when diagnosing a failing test.
All the other testcases in this file use "cmp" and I would prefer to maintain the consistency in this file. Note that the non-stylistic fixes (breaking up pipes) in this test were originally not the goal of this patchset, so changing this entire file to use test_cmp (or even just a single testcase) seems like it's just getting even farther away from the original goal of making pipe placement consistent in the code base.
Rather than escaping "$" with backslash, a cleaner fix would be to change the double quotes around the test body to single quotes. Those double quotes weren't needed anyhow since there are no variable interpolations in the body. Single quotes would make that obvious at a glance in addition to avoiding unexpected behavior in the future (like $1, $2, etc. being interpolated at the wrong time). Single quotes would also make the test more idiomatic and consistent with the bulk of other tests in the suite. If you do go the route of swapping quotes, please be sure to mention the change in the commit message.
Done. I thought of that earlier and thought that I should use double quotes for consistency with the surrounding code, but then I saw that there were testcases farther away in the same file that used single quotes. Here is the new commit message: t9109: don't swallow Git errors upstream of pipes 'git ... | foo' will mask any errors or crashes in git, so split up such pipes in this file. One testcase uses several separate pipe sequences in a row which are awkward to split up. Wrap the split-up pipe in a function so the awkwardness is not repeated. Also change that testcase's surrounding quotes from double to single to avoid premature string interpolation. Signed-off-by: Matthew DeVore <redacted>
Thanks.
Thank you!