Allow empty directories for --gather-test-outputs-in

[rrjjvv]

This addresses the usability issue from #594. The rationale for the previous behavior (not allowing the directory to exist) makes sense; test executions to the same to the same location, over time, aren't guaranteed to generate the same output files (in name or quantity), leading to older test outputs bleeding together with newer ones. That reasoning is still maintained.

This does the bare minimum. More could be done on the usability front, but it addresses my reasons for originally raising the issue: it matches my newbie expectation of what should work, it's consistent, and it's documented.

• I have reviewed the Contributor Guidelines.
• I have reviewed the Code of Conduct and agree to abide by it

[martin-schulze-vireso]

Regarding your usability concerns: we could offer a destructive version of that flag which does a cleanup first.

[martin-schulze-vireso]

I took the liberty to add --cleanup-and-gather-test-outputs-in <directory>. I'd like some feedback on the UX (and code) of that before merging.

[rrjjvv]

I'll give it a spin! I'm a tad busy, so might be a day or two before I can wire in this branch into my project.

I'm the last person you want giving feedback on (bash) code. At least for correctness. One of the things that drew me to this tool was how you hide away all the shell complexities (and very elegantly) as I always make the same mistakes over and over again. However, I can see one huge improvement (despite my next comment).

The flag won't actually be of use to me, but I'm probably not your typical user. In my current usage (that resulted in me raising this issue), this is running as a one-shot Job in a kubernetes cluster (asserting the behavior of a kubernetes component, from within the cluster itself). So every execution is in a pristine environment (no left-over files from previous executions). And most of my day-to-day activities are in ephemeral/containerized environments, so further adoption of bats would similarly be run in pristine environments.

So in my report, my preference (on top of explicitly documenting the requirements, which we've taken care of) was to either allow pre-creation of all directories, or prohibit presence of all directories. The runtime environment would always be created from scratch, so it just looked goofy being required to recreate some things, but prohibited from creating others. But I'm glad you were agreeable to the first (my original PR), as it tends to make things a little cleaner when it comes to 'exporting' the results onto volumes.

If the features involving generation of outputs has been (and will be) stable, I'm sure your new flag will be useful to someone. But if you anticipate more features, another option would be to make the concept a little more general: a --always-create/--never-create pair (that applies to all features that generate outputs). Or a --strict/--lax (where the first always clears folder contents, the latter leaves them untouched). Or be forgiving by default, and have a --ci (that would either require directories be empty, or would forcible delete the contents).

Of course, any of those would require more work (to apply those behaviors to the existing output mechanisms). And even if you find it a better approach, it still may not be "worth it" if the number of things you output is stable and everybody has been happy with the status quo, which is probably the case. Just wanted to throw it out there.

The error handling, assuming it does what I think it does, is a huge improvement! I think if that had been in place originally, I probably would have never opened up this issue. If my original error would have been "setting X requires Y" (rather than the raw mkdir error) I would have just followed those directions and gone on my merry way. It was figuring out what/why the failure was happening that led to this. I think that's actually the most valuable part of this entire change!

---

In summary:

1. Thanks for accepting my change!
2. Your new flag doesn't actually help me (but not to say it won't help someone else); it certainly doesn't hinder me.
3. The messaging/error handling is a huge improvement.

[martin-schulze-vireso]

I reverted the --clean-and-gather... change. I misunderstood your UX complaints and now concluded that this option is of too little use. My intention was to allow for easy rerunning locally. On a CI server dealing with non-empty directories is always an error and should be flagged accordingly. Locally, there is no big problem in prepending the cleanup manually to the bats command, so we don't win much.

[rrjjvv]

Yeah, I probably should have made my setup and usage patterns (CI/ephemeral environments) clearer in my initial report. Sorry for the confusion. I've pointed my project to directly use this commit (until it's formally released), and is working great.