On Thu, Nov 10, 2022 at 12:02 PM Rudy Rigot <rudy.rigot@xxxxxxxxx> wrote:
> > the <<- operator allows you to indent the here-doc body
> > (with TABs, not spaces), so you can align the body with the rest of
> > the code
>
> Unfortunately, that's how I had done it first, but since some of those
> lines are blank, the test code had lines just made of "<tab><tab>" and
> nothing else, which made the check-whitespace check fail. I considered
> replacing empty line with something on the fly with sed (like just an
> "x" character for instance), but this felt hacky and brittle (in the
> unlikely case where an actual "x" would find itself genuinely lost in
> the middle of that output, the test would mistakenly pass). I went
> with the solution I'm presenting here because the readability
> downsides of missing that indentation felt less bad. Definitely
> willing to be convinced though.
Okay, I see what you're getting at. Fortunately, there is a simple
solution as long as those lines are truly blank as emitted by `git
status`: just leave the blank lines completely blank in the here-doc
body (don't bother inserting a TAB on the blank line). This should
product the exact output you want:
cat >../expected <<-\EOF &&
On branch test
No commits yet
...
EOF
Although it should not be needed here, the `sed` approach is generally
fine, and we use it often enough in tests, though usually with a more
uncommon letter such as "Q". See, for instance, the q_to_nul(),
q_to_tab(), etc. functions in t/test-lib-functions.sh.
> > I presume the reason you're escaping the "trash" directory is because
> > you don't want these untracked "actual" and "expected" files to
> > pollute the `git status` output you're testing?
>
> You are presuming right! The test was being flappy in CI runs before I
> changed this, which I found used as a solution in other
> git-status-related tests currently in the codebase. I'm not familiar
> with the trash directory approach, but I'll figure it out.
Each test script is run in a temporary "trash" directory which gets
thrown away when the script finishes. We want tests to constrain
themselves to the trash directory so they don't inadvertently destroy
a user's files outside the directory.
I see what you mean about some existing status-related tests using
files such as "../actual" and "../expect". It's not at all obvious in
a lot of those cases but they are safe[*] because those tests have
already cd'd into a subdirectory of the "trash" directory, thus
"../actual" is referring to the "trash" directory itself, hence the
tests do constrain themselves to "trash".
Anyhow, I suspect that crafting a custom .gitignore file in the test
setup should satisfy this particular case and allow "actual" and
"expected" to reside in the "trash" directory itself without mucking
up `git status` output.
[*] Unfortunately, some of those scripts are poorly structured because
they `cd` around between tests, which can leave CWD in an unexpected
state if some test fails and subsequent tests expect CWD to be
somewhere other than where it was left by the failed test. These days,
we only allow tests to `cd` within a subshell so that CWD is restored
automatically whether the test itself succeeds or fails. So, this is
safe:
test_expect_success 'title' '
do something &&
mkdir foo &&
(
cd foo &&
do something else >../actual &&
)
grep foo actual
'
