Re: [PATCH] test-lib: some shells do not let $? propagate into an eval

[Jeff King <peff@peff.net>]

On Thu, May 06, 2010 at 03:41:10AM -0500, Jonathan Nieder wrote:

> I tried the cleanup_ret idea; test_run_ ended up looking like this:
> 
> 	test_cleanup=:
> 	eval >&3 2>&4 "$1"
> 	eval_ret=$?
> 	eval >&3 2>&4 ":; $test_cleanup"
> 	cleanup_ret=?
> 	(exit "$test_ret") && (exit "$cleanup_ret")
> 	eval_ret=$?

Maybe it is just me, but those last two lines would be much more
readable as:

  if test "$eval_ret" = 0; then
    eval_ret=$cleanup_ret
  fi

assuming your "$test_ret" was supposed to be $eval_ret.

> That breaks the principle of keeping the ugliness in test_when_finished.
> So here’s the minimal fix.

I don't know that we have necessarily have that principle. The most
important thing is that ugliness not escape to the test scripts
themselves, where we have to write many thousands of (hopefully
readable) lines.

But...

> -- 8< --
> Subject: test-lib: some shells do not let $? propagate into an eval

This fix looks fine to me, and I tested it on FreeBSD 8.0. So:

 Acked-by: Jeff King <peff@peff.net>

> I was also surprised to see this migrate to maint so quickly, but I
> was happy to see it broke early and loudly.

I think Junio is much more carefree with changes that only touch test
infrastructure, as they cannot possibly be breaking git itself for
anybody.

> Because there is some unhappiness with the feature[1], it might make

To clarify my position, I am not that negative on the feature. I just
think it is a bit of a fool's errand, and I don't want to personally
spend any time on making it happen. If you think it is worth proceeding
and you can do so without breaking anything[1], I won't object.

[1] Yes, I am teasing you. But probably the complex part is now done,
and you and others can use test_when_finished() at will now, and we can
see if it actually makes anybody's life better.

-Peff