The combinatorics seems to be as follows:

Improve the man page wording

It looks fine that atf-check wants to verify that its temporary dir has enough access rights as it needs to store output files beneath. Why does it add so many code to handle it explicitly? Probably, a usual noperm file system error does not propagate nicely and leaves a test result reader guessing about the actual root cause. Why doesn't it "escape" the current umask instead (as this patch proposes)? Maybe, there was another reason to obey the current umask when ATF was just a separate project before Kyua introduction and it was invoked directly. I do not see answers in the code parts I've read.

In D49463#1143637, @ngie wrote:

This feature can be refined, but I don’t want to get in the way of the utility of this change for folks like @kp, who need the debuggability. Let’s go forward with this change and refine things/sand down rough edges on GitHub.

Fix the man page format

I remember ngie@ agreed in some past cases to go directly to the src while the expected vendor path could go in parallel at its own pace. The vendor branch seems to be non-updated since its import back in 2020, so the vendor path may face obstacles and could take time. I mention this just in case if there is a need to hit src sooner, so that an exception agreement could be considered.

In D49798#1135647, @ziaee wrote:

for format/typo changes, I don't bump the date.

Sorry for the oversight, this is correct. We should not bump the date here.

Many thanks for review! It was a rush to meet the deadline, and there weren't enough rounds of self-check.

Fix format and style

It seems that as a temporary solution it is good to have them explicitly "tagged" to easily find the ones which need revising to avoid external dependencies.

In D49594#1131294, @lwhsu wrote:

I am happy to see allow_network_access feature has been added to the test framework. I believe it would be useful in some cases that network access is mandatory. However, in the case of DNS resolving, I suggest we just using the built-in local unbound for a simple local resolver to serve the test needs.

In D49594#1131294, @lwhsu wrote:

Sorry for being late to this (well, the window is quite short and I have been busy with other things recently...)

In D49594#1130770, @igoro wrote:

While ideally tests should not depend on external things as much as possible to have more reproducible nature, some of them may require to reach external resources, let's say to fetch something or to talk to the world services like DNS, etc.

Indeed and that's also why I am still hesitated to enable internet access on the jails in builder (when doing builds with the patches come from external) and when running the tests in VM. It's not only the security reason but also the matter of tests stability and reliability.

In D49594#1130777, @ziaee wrote:

We can do it later, but if you mark these variables up as It Va instead of It, they become searchable; e.g. apropos Va=allow_network.

While ideally tests should not depend on external things as much as possible to have more reproducible nature, some of them may require to reach external resources, let's say to fetch something or to talk to the world services like DNS, etc.

Add dch as a point of contact

Thank you, it was an interesting journey.

In D47668#1130374, @jamie wrote:

I'm not sure what the point of the soft/hard size limits is. I imagine I could find it in the code, but could you explain it to me? Something more than the one comment about it, but less detail than the code itself :-).

In D47668#1130373, @jamie wrote:

I don't disagree that the way the character set is limited is efficient and doesn't add much to the code base. It's just the entire direction that I don't like. There are many places where sloppy text interpretation can cause problems. Environment variables come to mind - they too are passed between processes in the kernel. But user-level string injection has never been considered a kernel problem, and I don't see why it should start now. That's just not the kernel's job.

I you want to continue to push for this, I would suggest separating it from the main feature, adding another revision just for that. I like the general meta/env feature, but you'll just have to look elsewhere for buy-in to that one part.

Remove meta_allowedchars mechanism

Thanks for the confirmation.

In D49463#1128242, @ngie wrote:

• Could you please verify that this doesn't regress Linux/MacOS?

Polish and apply the suggestions

Well, it was a quick one hour patch, now it's time for polishing.

In D49463#1128023, @markj wrote:

Whoa, nice. :)

What happens if tests are running in parallel? The behaviour I would naively expect is that other running tests will keep going, but new ones will not be scheduled until kyua is resumed.

Report test work dir

LGTM

In D48087#1117288, @ngie wrote:

Could requires be used instead of the term prepare? I think that would align the concept and the command more with what is being reported on.

In D47668#1120011, @jamie wrote:

I'm glad that turned out to be a workable option - thanks for adding it.

As to your comment that return an empty value for a nonexistent key instead of stopping the whole thing up with ENOENT, I see your point. But now I see another way: the NULL value, this time in the other direction. From the kernel interface side, it would be just the same as the setting direction, where receiving a NULL indicated attempted retrieval of a nonexistent key, while an empty string would only be for a key with an empty value.

It gets trickier on the user end though, where jls(8) only sometimes allows for a distinction. In particular, "jls -n meta.foo" or "jls -s meta.foo" could be made to show the same distinction between meta.foo="" and just meta.foo, but without the option there would be no way to tell. That's probably sufficient for those who care, though.

Communicate back a missing key

In D47668#1115164, @jamie wrote:

A few observations from actually running this:

Add key removal mechanism using NULL value as a trigger

Add the respective code comment

In D49039#1118150, @asomers wrote:

LGTM. Except that the commit message contains a typo: "uprivileged_user" => "unprivileged_user"

I guess it could be helpful for a couple of test cases from tests/sys/netinet/fibs_multibind_test.c and/or tests/sys/netinet/socket_afinet.c.

@ngie , it would be appreciated if you could find time to consider this. It seems that your opinion is necessary to move forward.

Update kyuafile.5 after landing of D48190

Improve the comment regarding the retry mechansim

tests: Use recently added support of is.exclusive coming from ATF side

This is a follow-up to https://reviews.freebsd.org/D48190, where it was discovered that this support is missing. It's obvious that our test suite does not use it, and this patch is for consistency and not to say "not mapped" in the kyuafile.5. Also, I found that Kyua 0.11 release notes state that it should work, so actually this is a fix.

In D48190#1100132, @ngie wrote:

I think submitting this upstream (freebsd/kyua) and filing an issue capturing the feedback/concerns would probably be ok.

In D48087#1096440, @kp wrote:

One trivial remark about 'pip? # e.g. to have scapy installed'. We have a scapy package and that's how I've always installed it. Right now it's py311-scapy-2.6.1.

In D48190#1099037, @ngie wrote:

It honestly seems like this documentation should live in another place, e.g., a "test driver" [1] (kyua-atf-driver(4)?) manpage. Putting all of this information in a single manpage seems like it would really clutter up what's being explained here, and the more "test drivers" get bolted on to kyua, the more complex the documentation will become.

1. I'm not sold on the name "test driver", but it's the best I can offer for the concept right now.

This is a follow-up to https://reviews.freebsd.org/D47671, where we discussed documenting of the mapping.

In D47671#1098794, @ngie wrote:

If you could commit it directly to the freebsd/src:main, that would be helpful :) (I think it's better to get it in sooner so it gets some bake time on FreeBSD).

Now the wording is away from the internals and closer to the end users:

> ./kyua help | grep prepare
  prepare                 Prepare env and resolve requirements before testing.

My current favorite is the prepare term proposed by Kristof. So, I've changed it in the latest version of the patch for the sake of further testing and discussion. Everything is open for renaming. The internals still use requirement resolver terminology, I could not quickly find the replacement and probably it's not needed due to "prepare" is kind of higher level, anyway this is the encapsulated part with the only exposure via CLI help output.

Rename "kyua rr" to "kyua prepare"; Drop "kyua test --rr"

Make keyvalue_contention test case more accurate

Tiny code improvements, no functional change

Add keyvalue_contention test case

Fix jm_h_cut_occurrences() logic

libjail: Correctly differentiate no<boolparam> from keyvalue-based ones

Each buffer can be handled as a set of key=value\n strings