Inconsistent directory requirements for --output vs. --gather-test-outputs-in flag

[rrjjvv]

Describe the bug
Disclaimer: I'm choosing to file this as "bug" because it (subjectively) fails "principle of least surprise" and I was unable to find documentation to explain (or hint at) the behavior.

I am simply trying to capture both test results and the outputs together. My desire was to capture both in sibling directories (i.e., a common parent). With no (obvious) documentation stating otherwise, I guessed/assumed creating the common parent directory would be sufficient and the two children would be automatically created. That resulted in an error. I then proceeded to pre-create both children directories. That also resulted in an error.

To Reproduce
Steps to reproduce the behavior:

I thought it might be clearer to assert the undesirable behavior and allow the test names to tell the story. So in this case, a passing test is actually a negative thing 😄 .

Contents of wut.bats:

#!/usr/bin/env bats

setup() {
  export DUMMY_TEST="$BATS_TEST_TMPDIR/test.bats"
  export STUFF_ROOT="$BATS_TEST_TMPDIR/stuff"
  export REPORT_DIR="$STUFF_ROOT/report"
  export OUTPUT_DIR="$STUFF_ROOT/output"

  cat <<'EOT' | sed 's/^ATtest/@test/' > "$DUMMY_TEST"
ATtest "simple" {
  echo "hi"
}
EOT
  mkdir "$STUFF_ROOT"
}

@test "no pre-created dirs fails (reasonable)" {
  run bats --report-formatter junit --output "$REPORT_DIR" --gather-test-outputs-in "$OUTPUT_DIR" "$DUMMY_TEST"
  [ "$status" -ne 0 ]
  echo "$output" | grep "Output path $REPORT_DIR is not writeable"
}

@test "pre-created report and output dir fails (unexpected)" {
  mkdir "$OUTPUT_DIR" && mkdir "$REPORT_DIR"
  run bats --report-formatter junit --output "$REPORT_DIR" --gather-test-outputs-in "$OUTPUT_DIR" "$DUMMY_TEST"
  [ "$status" -ne 0 ]
  echo "$output" | grep "can't create directory '$OUTPUT_DIR': File exists"
}

@test "pre-created report dir (only) succeeds (inconsistent)" {
  mkdir "$REPORT_DIR"
  bats --report-formatter junit --output "$REPORT_DIR" --gather-test-outputs-in "$OUTPUT_DIR" "$DUMMY_TEST"
}

@test "pre-created output dir (only) fails (inconsistent)" {
  mkdir "$OUTPUT_DIR"
  run bats --report-formatter junit --output "$REPORT_DIR" --gather-test-outputs-in "$OUTPUT_DIR" "$DUMMY_TEST"
  [ "$status" -ne 0 ]
  echo "$output" | grep "can't create directory '$OUTPUT_DIR': File exists"
}

Incidentally, this actually shows a second pseudo-bug: using the @test marker in a heredoc still gets picked up as a test. The sed hack was to avoid getting an "expected X tests but ran Y" warning. I'm assuming this is an extremely atypical pattern, so didn't think it warranted filing an issue. But I'd be happy to file a report if you feel it should be addressed.

Result (current and previous version):

$ docker run --rm -it -v $(pwd):/code:ro  bats/bats:1.7.0 wut.bats
wut.bats
 ✓ no pre-created dirs fails (reasonable)
 ✓ pre-created report and output dir fails (unexpected)
 ✓ pre-created report dir (only) succeeds (inconsistent)
 ✓ pre-created output dir (only) fails (inconsistent)

4 tests, 0 failures

$ docker run --rm -it -v $(pwd):/code:ro  bats/bats:1.6.0 wut.bats
 ✓ no pre-created dirs fails (reasonable)
 ✓ pre-created report and output dir fails (unexpected)
 ✓ pre-created report dir (only) succeeds (inconsistent)
 ✓ pre-created output dir (only) fails (inconsistent)

4 tests, 0 failures

Expected behavior
I would expect one of the following:

1. both directories required to be already created
2. requiring that neither directory exist
3. documentation explicitly stating the prerequisites (ideally, with an explanation for the discrepancy, if it's by design)

Environment (please complete the following information):

• Bats Version: 1.6.0 and 1.7.0 (but likely every version from when those features were first introduced)
• OS: Linux (my initial encounter, as well as my reproduction, were in containerized environments)
• Bash version: 5.1.16 (version present in the bats docker image)

Additional context
I am relatively new to this tool and have yet to explore all its features. Maybe, with extended use, it would be obvious why it behaves the way it does. If I was confident that it was a bug and that there was a "correct' answer, I likely would have contributed a PR alongside this report. But I'm not, so I didn't.

[martin-schulze-vireso]

That heredoc problem is known, see #277. I need to look into why the Flags handle existing directories differently but I would assume its just an oversight.

[rrjjvv]

That heredoc problem is known

My bad. I uncovered that totally on accident (and only due to creating a self-contained reproduction). It's unlikely I would have encountered it during real-world usage.

but I would assume its just an oversight.

As you'll discover, there is one "obvious" easy solution to make those two consistent with each other. But it was my lack of usage of all the other flags (that use directories), and not knowing if there was an underlying philosophy, that stopped me from submitting the obvious/easy two-character PR.

While it was probably implied, I should have explicitly added that this is not at all a blocker. It took me a while to figure out the issue and solution (for reasons unrelated to bats itself), but both flags are fully usable once you know the trick.

[martin-schulze-vireso]

After having looked into the issue I am more certain that this is not an oversight. The rationale behind requiring the folder not to exist is that you don't get mixed up outputs from a previous run with the current one.

This problem is mainly due to the fact that a report is a single file which can be replaced. While the outputs are one file per test, so you might get only partial overwrites.

This issue might be solved by better documentation or by a warning for a non empty directory.

[rrjjvv]

That rationale makes perfect sense in the context of allowing reports to be be written to a populated directory and refusing to write test outputs to a populated directory. But I would think an empty directory would be effectively "the same" in regards to the risks you mentioned...?

So right now my script has

# https://github.com/bats-core/bats-core/issues/594
# This *must* be pre-created, or things break
mkdir /results/report
# No-op, but reminder to *not* pre-create this, or things break
rm -fr /results/output
bats --timing --report-formatter junit --output /results/report --gather-test-outputs-in /results/output .

Maybe in that context you'll see why it's a little awkward. (Or maybe not.) But yes, even if you decide the behavior should remain as-is, calling out the existence/non-existence requirements in the docs (and/or warnings/diagnostics to the errors) should help anyone else that gets confused. Thanks!

[martin-schulze-vireso]

You are welcome to create a PR for the empty directory check.

[martin-schulze-vireso]

Fixed in #603