In D59276#1360539, @markj wrote:

My only concern is that it might eliminate some randomness from the test scheduling that helps probabilistically catch problems. I have not looked at all at the patch yet, nor at the existing scheduler: how does the patch affect determinism in the test scheduler?

Remove debug code leftovers, make execplan=none do nothing with profiles, revise licensing

In D59346#1361743, @ngie wrote:

@igoro when the time comes, could you please submit this upstream as well along with the prepare support?

LGTM. The funny thing is that originally it was h.

Probably a simple parameter like in the following patch could be good enough and a bit more descriptive: https://reviews.freebsd.org/D59067.

I've stumbled upon that if (npkt >= 20) condition, and now I recall why it's there. Originally, this app was created to test a fix of a defect where a packet could end up in a loop. So, the divert-to.sh test sends a single packet (ping -c1) and the divapp is programmed to catch not only the first one and break a loop eventually. This also explains the strict check at the end to expect a single packet only, if (npkt != 1).

In D57316#1315185, @ngie wrote:

In D57316#1314576, @des wrote:

In D57316#1313745, @ngie wrote:

This "fixes" the issue, but the tests should also be intelligent enough to understand when the OPENSSL_KTLS feature isn't present and skip the tests if the feature isn't present in the kernel (see ATF_REQUIRE_FEATURE, etc).

ATF_REQUIRE_FEATURE is awfully unidiomatic, we should implement a require.features property instead.

I agree.. just not sure how to handle it yet with other platforms where this feature might not be available. Probably should just push forward with the OS-specific handler behavior in kyua for FreeBSD started by @igoro .

It seems the main goal here was not about parallelism, so maybe it's not so important, but I just wanted to mention that the newly added test cases are still marked as exclusive as the Makefile makes it so for all of them:

# cat /usr/tests/sys/net/routing/Kyuafile
-- Automatically generated by bsd.test.mk.

In D55859#1278506, @glebius wrote:

Thanks a lot for working on that! I miss the feature a lot.

Sorry if a dumb question. Trying to understand the Makefile logic. The statement

${PACKAGE}SCRIPTS+=     demo.setup \
                        demo.atfsh_like_setup

does it apply to all tests in the Makefile or there is some magic that makes the first word before the dot to limit application only to a test that is named as this word? Where does this logic live?

There is an additional idea to think of in the future to add a new metadata named like execenv_jail_attachto=<jail_name> or so. The goal is, if an external setup configures a complex set of jails and the test is expected to be running within a specific child jail only, then such metadata would ask Kyua to do so.

The idea is to have an external setup/teardown files which could be implemented much simpler compared to the test itself. For example, setup is done with usual shell and tools from the base, while the test itself could be written in C.

@ngie , is anything else required to be changed or we are ready to land it?

@ngie , it seems we could land it, and copyright topic could be revised for entire project with separate commit(s). What do you think?

Throw std::system_error instead of std::runtime_error upon kldload failure

Explicitly describe execenv involvement in "kyua help debug"

Make it skip writing stdout/stderr to tmp files, as "debug -p" does

Communicate kldload issues via exceptions

In D52642#1205688, @markj wrote:

This works almost exactly how I want, thank you. I have one "complaint": when I use this mode, kyua 1) drops me in the shell, 2) waits for the shell to exit, 3) prints the debug output, e.g., executed commands and the test failure string. The -p mode is basically the same. It is more useful to do 3) first, then 1) and 2). Is it hard to make this change?

Resolved review comments

Hi @ngie. It seems your time budget is very limited. Thank you for all the comments you provided for this patch, it helped polishing the things and making it better. I'm going to land it soon so that we could start actual testing of the idea. Fortunately, it's an opt-in helper tool for a developer and nothing depends on it. Anyway, before it can be useful we need to continue labelling the tests with required_kmods declarations.

LGTM. The same testing passed on my side, with the same build and test env. The only obstacle I had is this:

In file included from netlink_netlink_snl_route_parsers.c:1:
In file included from /home/igoro/src/sys/netlink/netlink_snl_route_parsers.h:30:
/home/igoro/src/sys/netlink/netlink_snl.h:122:12: error: comparison of integers of different signs: 'int' and 'uint32_t' (aka 'unsigned int') [-Werror,-Wsign-compare]
  122 |         if (delta > lb->size - lb->offset)
      |             ~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~
1 error generated.
*** [netlink_netlink_snl_route_parsers.o] Error code 1

Implement x/y flaky spec

In D54148#1237012, @jhb wrote:

Igor, are you able to test this again on your armv7 setup btw?

LGTM. That's an important point regarding the other data allocated from the snl_state.

It seems to be ready for landing to slowly start actual testing and adoption.

In D53697#1226388, @jhb wrote:

@igoro would you be able to test this on your workload (armv7) to ensure it still does the correct thing?

Now I remember that the latest fix's key goal was to add missing nw->size maintenance not to realloc again-and-again when it's actually not needed, i.e. after each realloc we still think that nw->size is only 256. And the motivation was to avoid overlapping buffers appeared to be there as a consequence of missing nw->size maintenance.

In D52910#1211188, @ivy wrote:

i'm not opposed to this, but i don't understand why it's required. is this an upstream bug, or are we building libkrb5 wrongly?

Thanks for your time and review.

Tiny improvements

This seems to be revealed with the recent switch to MIT version. For example, 15-CURRENT back in March had this:

# ldd /usr/lib/libkrb5.so
/usr/lib/libkrb5.so:
        libasn1.so.11 => /usr/lib/libasn1.so.11 (0x36e32d570000)
        libcom_err.so.5 => /usr/lib/libcom_err.so.5 (0x36e309956000)
        libcrypt.so.5 => /lib/libcrypt.so.5 (0x36e30a44a000)
        libcrypto.so.30 => /lib/libcrypto.so.30 (0x36e30a600000)
        libhx509.so.11 => /usr/lib/libhx509.so.11 (0x36e339780000)
        libroken.so.11 => /usr/lib/libroken.so.11 (0x36e30ac5a000)
        libwind.so.11 => /usr/lib/libwind.so.11 (0x36e33bec0000)
        libheimbase.so.11 => /usr/lib/libheimbase.so.11 (0x36e30834e000)
        libprivateheimipcc.so.11 => /usr/lib/libprivateheimipcc.so.11 (0x36e30ba76000)
        libc.so.7 => /lib/libc.so.7 (0x36e308e00000)
        libmd.so.7 => /lib/libmd.so.7 (0x36e30c885000)
        libthr.so.3 => /lib/libthr.so.3 (0x36e30d741000)
        libsys.so.7 => /lib/libsys.so.7 (0x36e318150000)

The prototype has the following logic currently:

Always attempt to use $SHELL

In D51136#1196294, @ngie wrote:

In D51136#1170880, @igoro wrote:

In D51136#1170613, @ngie wrote:

What was the behavior prior to this change?

It's turned out that kyua test catches issues with temporary dir/files cleanup but does not propagate it -- that's obvious from the code where test_result is not used actually. As a result, a test is marked as passed leaving garbage behind (tmp dirs/files), while by design and code comments it is expected to be broken instead.

Will this cause false positives with cleanup failures?

Rebase onto the latest kyua changes and rework from rr to prepare

A minuscule improvement to get the following upon atf_check_equal failure:

Error Message:
1 != $(jexec alcatraz sysctl -n net.dummymbuf.hits) (1 != 2)

instead of this:

Error Message:
1 != 2 (1 != 2)

LGTM

Thank you very much for identifying the root cause. I've been monitoring the false positives on the CI since Oct-2024 -- 25 of them so far. And my TODO list is too deep.

In D51136#1170613, @ngie wrote:

What was the behavior prior to this change?

LGTM. And I had a successful testing on my side.

LGTM

In D51035#1165778, @ngie wrote:

Please submit this upstream. We need to work through the porting issues with Linux/MacOS in order to land this to avoid having multiple different forks/mutations of this (ATF) code.

LGTM

The combinatorics seems to be as follows:

Improve the man page wording