Re: [PATCH 12/12] test-lib-functions: split out {debug,path,text} helpers

On Thu, Feb 11 2021, Johannes Schindelin wrote:

Hi,

On Tue, 9 Feb 2021, Junio C Hamano wrote:

quoted

Ævar Arnfjörð Bjarmason  [off-list ref] writes:

quoted

Split out the debug, path and text miscellaneous helpers into their
own files. The "test -[efsd]" etc. helpers were not all in one place
in test-lib-functions.sh, I think moving them into their own file
makes it easier to follow the code.

I'd prefer not to see this done.  It would be fine to move them in
different sections but still in the same file to group related
helpers together, but I am having enough trouble when deciding to
look into which between test-lib.sh and test-lib-functions.sh in the
current organization already.  And it will make the situation even
worse if we did this, no?

As much as I would love to see a better organization, I agree with Junio
that this split would make it more tedious to find the correct function.

I had a look over the rest of the series and like what I saw, leaving only
one or two comments.

Junio / SZEDER / Johannes: Yeah I agree that that 12/12 sucked in its
current form.

Having eyeballed test-lib.sh and test-lib-functions.sh as part of this
though there seems to be a pretty obvious way to split this up which I
think would increase the readability:

 * test-lib.sh: main entry, reading/setting global vars/state,
   command-line parsing (and other CLI / Makefile interface), loading
   test-lib-functions.sh, set global prereqs, actual setup for the test
   about to be run.

 * test-lib-functions.sh: the top-level "library" used everywhere,
   i.e. test_expect_success/test_expect_failure etc, Hopefully we could
   move the internal functions like test_ok_, test_failure_ etc. from
   test-lib.sh to test-lib-functions.sh (they need some global variables
   in test-lib.sh now).

   Also things part of core test "flow" like test_config,
   test_when_finished, nongit etc.

 * lib-common.sh: what I was trying to start with in this 12/12,
   i.e. utility functions common enough that we want them
   "everywhere". We'd just have test-lib-functions.sh load this.

That seems like a much more obvious split & organization to me than the
mis-match with a bit of everything everyhere we have right now.