Oh, thanks, all of this helps a ton. I know exactly what to do about those two things now. On Thu, Nov 10, 2022 at 11:30 AM Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote: > > 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 > '
