Update remaining version numbers I missed in prior revision.

Update the remaining version numbers/strings

Double-submitted as D49296.

Update with the changes in the tree instead of the vendor import commit

This ideally should have a test added for it (along with the other ATF <-> Kyua variable parity improvements you made).

FWIW, I think the real bug might be that net/scapy requires python3 instead of being properly flavored using PY_FLAVORS, or the shebangs not being updated in the port to use the python3.10 instead of python3.

In D49007#1118213, @kp wrote:

In D49007#1118206, @ngie wrote:

It should replace existing lines where scapy is required. Please use your best judgment about whether or not it should be expressed this way.

I meant that to ask about what atf will do.
I.e. given this:

some_head()
{
    atf_set require.progs foo
    require_scapy
}

do we end up requiring python3 and scapy only, or foo as well?

One thing I might do instead of “requires_scapy” is something like “netpfil_required_progs” or set the requirements at the Kyuafile level. Again, this would dramatically reduce the amount of redundant shell code in the ATF testcase headers.

I wish the metaport stuff was handled like Debian sisters do with the alternatives application :/..

In D49007#1117997, @kp wrote:

In D49007#1117529, @asomers wrote:

What maintenance burden, @kp? I think it's a worthwhile change.

I'm probably overstating it a bit, yeah. I mostly want to keep the tests as simple and straightforward as possible, and having to list what should be a dependency of your already listed dependency isn't that.
Still, it's small enough that I'm not going to fight against it. Put me down as a "doesn't actually say no".

I've run into similar problems myself. I find it easy to have a system where something pulls in, say, lang/python311 but not lang/python3.

Why the python ports do this is probably off-topic here, and not something I'm going to be trying to fix.

In D49007#1118128, @kp wrote:

In D49007#1118107, @jlduran wrote:

In the long, I can propose something like:

require_scapy()
{
    atf_set require.progs python3 scapy
}

In the common subroutine, and use require_scapy in the header, if the verbosity is the burden.

Interesting idea. Does that extend or replace previous atf_set require.progs lines? (You may not know, I certainly don't. Something we need to test if we go that way.)

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.

The kyuafile solution would have involved less change, but I get the merits/demerits of going that route vs injecting the metadata in the _head() functions.

Sidenote: you can require the program in the Kyuafile as well. That's generally the best way/least amount of code to ensure that the requirement is actually being met.

What is the benefit of doing it this way vs the other way using require.progs?

In D48190#1099473, @igoro wrote:

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.

Agree, and it seems that we could go with a much more useful reference "just in place" for now and re-work it when it's time to cover a couple or more drivers. For now ATF & Kyua are very coupled from the perspective of our test suite, and it seems that lack of such quick reference does not allow test authors to squeeze maximum from both frameworks -- kyuafile.5 already states that ATF test case could override Kyua metadata, but it may be left non-obvious for a reader how exactly ATF naming is mapped. It could be guessed by comparing with atf-test-case.4 but the things like require.machine vs allowed_platforms can be tricky and need experimenting to make sure. Another issue is that Kyua could read Kyua-specific metadata from ATF side even if ATF knows nothing about it and it's not documented anyhow respectively, e.g. execenv and execenv_jail_params.

What do you think?

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.

In D47671#1095970, @igoro wrote:

In D47671#1095969, @ngie wrote:

Thanks! Please update the differential revision with the tests. Memory serves me correctly, CI on main might start failing if the tests aren't introduced.

Thank you for support. Kyua's tests are not configured yet on the FreeBSD side (in my todo with low priority). I would propose to keep this commit simple as it is right now to have less conflicts when it's time to merge from the vendor branch. I'm totally fine to add the test code change, but it can diverge more before the eventual merging. I guess we already have a lot of merge fun piled up there, I'm just worried about the person who is going to do the merge :).

Do you accept this approach?

In D46653#1092361, @markj wrote:

In D46653#1091544, @ngie wrote:

I think being added to the FreeBSD org is documented somewhere in the Committer's Guide. CC: @markj

I don't see any docs on that. Am I missing something?

In any case, I submitted https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=283127

Regarding the change itself, I have a hard time believing any serious project is parsing the result summary line and using that as input to something else. Do we know of any actual examples?

I kind of thing option 4 makes the most sense..

For the run of the FreeBSD regression suite that I just completed, option 4 gives:

Test cases: 9032 total, 211 skipped, 33 expected failures, 1 broken, 6 failed

Whereas kyua currently prints:

8814/9032 passed (1 broken, 6 failed, 211 skipped)

In D47671#1095956, @igoro wrote:

Thanks for reminding about testing. Please, find the PR here: https://github.com/freebsd/kyua/pull/245.

Please open a PR against freebsd/atf freebsd/kyua.
Also: this deserves a test (upstream).

LGTM!

In D46653#1091680, @igoro wrote:

In D46653#1091544, @ngie wrote:

Regarding the change proposed on the GitHub PR. I think it's better to merge the existing PR to keep FreeBSD src/contrib/kyua and the github/freebsd/kyua in sync with absolutely the same commit. And I would open a new PR for the change requested with a reference to the previous GitHub discussion/PR. What do you think is the best here from the organizational perspective?

I don't think it would be a good idea to merge the PR as-is. I'm trying to avoid violating POLA as much as humanly possible for 0.14 and taking a look at the output definitely violates POLA. There are enough Linux/MacOS things to deal with -- I'd rather not get more questions with a UX change like has been made on main...

I forgot to mention explicitly that I was thinking about "for the sake of easy merging between FreeBSD and github/Kyua". Now, as I get it, you worry about potential parsers of the last line of the output. Yes, I mentioned it during the discussion here, and it was like there are no objections to changing it. But, having it changed only for FreeBSD, we could step back and re-think the interface again, before GA.

I would like to list the options that quickly come to mind:

1. Revert the change [1], keep UI as before.
2. Revert the change, and probably add an extra line above the last one:

1 passed, 221 skipped, 0 broken, 0 failed, 222 total
222/222 passed (0 failed)

3. Change the last line to:

1 passed, 221 skipped, 0 broken, 0 failed, 222 total

4. Change the last line to repeat output of kyua report --verbose:

Test cases: 222 total, 221 skipped, 0 expected failures, 0 broken, 0 failed

5. If we decide to astonish the users and change the last line then we could think of an extra value for exchange, e.g. make it provide some "smart" summarization to highlight the majority of the results. Perhaps I went too far, or maybe it's just a cup of great coffee talking here :). Please, do not take my wording as the best one:

All failed: 0 passed, 0 skipped, 1 broken, 221 failed, 222 total
Mostly failing: 90 passed, 0 skipped, 1 broken, 131 failed, 222 total
Some failures: 200 passed, 0 skipped, 2 broken, 20 failed, 222 total
All skipped: 0 passed, 222 skipped, 0 broken, 0 failed, 222 total
All passed: 222 passed, 0 skipped, 0 broken, 0 failed, 222 total

[1] Revert the change in FreeBSD main and stable/14 branches. Close the GitHub PR w/o merging.

Please let me know your vision/plan when you have one, and which steps are mine -- I'm ready to help with any option.

In D46653#1091534, @igoro wrote:

In D46653#1091513, @ngie wrote:

For the record, stuff like this should really be committed to freebsd/kyua first, then backported. As of right now this change is not in what's intended to go in to 0.14 and in order to do the upgrade I would need to reapply this patch. I provided a ship-it earlier, but I was swayed by the comment in the PR about this change being confusing for naive results parsers: https://github.com/freebsd/kyua/pull/230 .

Yeah, ideally it should go through the GitHub repo. I tried to help with the synchronization by keeping this review and the GitHub PR in sync, the latter was expected to be "blindly" merged after all approvals on the Phab, unfortunately I do not have access to do that. Probably, we could think of some access for me? My vision is that I could merge the things like that PR after all discussions and approvals on the Phab side.

For the record, stuff like this should really be committed to freebsd/kyua first, then backported. As of right now this change is not in what's intended to go in to 0.14 and in order to do the upgrade I would need to reapply this patch. I provided a ship-it earlier, but I was swayed by the comment in the PR about this change being confusing for naive results parsers: https://github.com/freebsd/kyua/pull/230 .

Mostly a style(9) nit: style(9) says C comments should be used (/* ... */)

Please rebase / reapply the new code changes. I reformatted the sources with clang-format in rG991bd461625a2c521d5be4fd6938deed57f60972 so your proposed changes will be highlighted instead of the current diff--which is addressing some existing style bugs in addition to your functional changes.

Let me fix the formatting issues beforehand so the diff can be minimized to the functional changes...

Will go through openzfs first.

As noted elsewhere, this unfortunately doesn't work for UID != 0: https://github.com/openzfs/zfs/pull/16820#issuecomment-2508540991 .

Could you please run clang-format on this file to resolve any formatting-related issues?

In D40366#1076183, @brnrd wrote:

In D40366#1076061, @ngie wrote:

Install the dbm module for OPTIONS_SET+= BDB1 to unbreak uses of the dbm submodule.

Not used phabricator for a good while (last was before svn->git?)... WebUI doesn't want to update this rev.

I think I've cracked it, the plist breakage and all. Check [https://brnrd.eu/bsd/patch-devel_apr1-D40366]
Basically:

1. --with-dbm hints for configure to speed up bdb detection
2. Switch to PLIST_SUB with BERKELEYDB for all BDB versions

Database creation using httxt2dbm works with all 3 BDB versions:

❯ file test-*.dbm
test-base.dbm: Berkeley DB 1.85 (Hash, version 2, native byte-order)
test-db18.dbm: Berkeley DB (Hash, version 10, native byte-order)
test-db5.dbm:  Berkeley DB (Hash, version 9, native byte-order)

Would love to switch default to base, but that'll trip up people that have existing databases.

In D46806#1067801, @kevans wrote:

In D46806#1067666, @ngie wrote:

Could you please demo the test program (attach the kyua html report)?

Neat, I had no idea that kyua had a report-html command.

I uploaded the report here: https://people.freebsd.org/~kevans/porch-results/

archive.tar in that directory packages the directory for convenience. IMO it leaves a little bit to be desired, I would've kinda liked to see test names to attribute each result to, but maybe it does if they fail/skip.

Install the dbm module for OPTIONS_SET+= BDB1 to unbreak uses of the dbm submodule.

Oh neat, the plist is broken for all but OPTIONS_SET+= BDB.

In D40366#1073408, @brnrd wrote:

How did you verify that Berkeley DB support is working with these changes? httxt2dbm is failing for me.

In D46653#1064186, @igoro wrote:

Rework structure of test result types metadata. Fix tests.

Rebase on recent ports tree

Update the diff to the latest version: 1.7.5