testsuite: don't assume /bin/true and /bin/false

[garlick]

Problem: tests that hard code /bin/true or /bin/false fail on macos, where those programs are in /usr/bin.

When PATH is already being searched, don't specify a fully qualfied path.

[chu11]

Would going with /usr/bin/true work? Appears to be ok for RHEL9 and my ubuntu laptop at home. Without any context, just seeing true and false, it looks like it could be boolean. Like

test_expect_success 'rc3 failure causes instance failure' '
	test_expect_code 1 flux start \
		-Sbroker.rc3_path=false \
		-Slog-stderr-level=6 \
		true 2>rc3_failure.log
'

like false could be a special option to rc3_path.

Maybe it's just me.

[garlick]

Oops, pushed a fix to the ASAN error - I inadvertently changed a string length in the broker runat test without changing another value that was supposed to be its length + 1.

Without any context, just seeing true and false, it looks like it could be boolean

That would probably work although I think it's pretty clear what's going on in these tests. I'd be inclined to not continue to hard code a path - it feels slightly wrong.

[chu11]

That would probably work although I think it's pretty clear what's going on in these tests. I'd be inclined to not continue to hard code a path - it feels slightly wrong.

By all means, it certainly livable. Just something I noticed at first glance.

[garlick]

Maybe we ought to get @grondo's input on that one.

[grondo]

This approach seems fine to me. To be fair, we have some other paths we hard code in the testsuite including /bin/sh, /bin/bash, and /bin/echo. I'm guessing those are ok because they exist in those paths on macos?

[grondo]

One idle thought: Since use of /bin/true /bin/false is fairly ingrained AFAIK, I'm guessing we'll end up accidentally breaking macos with future commits. I suppose we'll automate detection of that breaking in the future when we can enable make check , but since we can't do that now, we'll have to just be fine with updating /bin/{true,false} in the future when make check does work on macos. I guess that just makes me wonder if fixing this once would be better?

I'm actually fine either way, maybe we won't end up re-introducing these errors. (the larger point being it is slightly preferred to add a check for an issue when fixing that issue so some lame future committer (likely me) doesn't re-break what has already been fixed)

[garlick]

These are all test changes, so I think the rule of "add a test when you fix something" probably doesn't apply or at least not as strongly. And the person who finally gets macos/bsd to run all the tests may have to fix whatever pops up between now and then, but I think that'll be the least of their worries.

If we do it now, then at least they will have a lesser job, and the cut and pasters won't make the problem worse.

[garlick]

This approach seems fine to me. To be fair, we have some other paths we hard code in the testsuite including /bin/sh, /bin/bash, and /bin/echo. I'm guessing those are ok because they exist in those paths on macos?

Yeah, the problem is "fails on macos" and I wanted to keep the solution focused on that.

I'm seeing thumbs up and this is approved so I'll go ahead and set MWP. Thanks!

[grondo]

Yeah, the problem is "fails on macos" and I wanted to keep the solution focused on that.

I figured. My point was it went against the "it's wrong to hard code a path" argument made previously in the comments.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.64%. Comparing base (2ddf0d5) to head (cd2ef59).
Report is 3 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #6507       +/-   ##
===========================================
+ Coverage   53.72%   83.64%   +29.91%     
===========================================
  Files         475      525       +50     
  Lines       79573    87693     +8120     
===========================================
+ Hits        42751    73350    +30599     
+ Misses      36822    14343    -22479     

see 446 files with indirect coverage changes