Page MenuHomeFreeBSD

kyua: Support is_exclusive metadata coming from an ATF test case
ClosedPublic

Authored by igoro on Nov 19 2024, 1:14 PM.
Tags
None
Referenced Files
F107085387: D47671.diff
Thu, Jan 9, 9:24 PM
Unknown Object (File)
Mon, Jan 6, 10:36 AM
Unknown Object (File)
Tue, Dec 24, 8:40 AM
Unknown Object (File)
Dec 9 2024, 5:44 PM
Unknown Object (File)
Dec 5 2024, 3:13 AM
Unknown Object (File)
Dec 3 2024, 8:49 PM
Unknown Object (File)
Dec 3 2024, 4:00 PM
Unknown Object (File)
Nov 30 2024, 4:50 AM
Subscribers

Details

Summary

On ATF side it is named "is.exclusive".

MFC after: 1 month

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 60684
Build 57568: arc lint + arc unit

Event Timeline

igoro requested review of this revision.Nov 19 2024, 1:14 PM

I would like to share my thoughts regarding this.

To be honest, it does not feel like an easy change from a high-level perspective. ATF itself does not support the exclusiveness concept, and at the same time, at least atf-sh is okay with having unknown metadata, so that only Kyua will read it and deal with that. But this is what we have, our toolset is a mix of ATF and Kyua, and both of them could be treated as a single tool many existing tests are based on.

I had an idea to add this like a year ago, but I had no real use cases. And now I have an example of a single shell test program with multiple test cases, where most of the cases can be run in parallel (and even with execenv=jail), but some tests need to be exclusive, while there is no way to mark only specific test cases as exclusive. It's about jailmeta:maxbufsize test case here: https://reviews.freebsd.org/D47668. I might need to split max buf size cases onto separate test cases but I have to keep them together as a single test case for now with custom jail name to avoid clashing. And adding a separate shell script just for that looks like a redundancy, especially when some subroutines need to be shared.

The change itself looks fine to me, but don't we need to document this somewhere?

This revision is now accepted and ready to land.Nov 20 2024, 3:54 PM

The change itself looks fine to me, but don't we need to document this somewhere?

Thanks for raising this. To be honest I was thinking about it and decided to postpone such things due to the dilemma -- Kyua and ATF are separate projects and also very close to each other in our case.

As long as Kyua has tight integration with ATF then, probably, we could augment its man page(s) to explicitly refer to the corresponding ATF naming. For example, kyuafile(5) could have ATF name reference per each metadata property, whether it is something ATF supports but names differently or ATF-based test cases can pass Kyua specific properties like "is exclusive". It seems to be useful to have a single place for Kyua metadata description with translation to ATF language, instead of a separate man page.

If there is support for such approach then I could propose to do that as a separate patch. What do you think?

ngie accepted this revision.EditedFri, Dec 13, 8:37 PM

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

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

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

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

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.

Please also add "MFC after: 1 week" to this commit -- it should be backported to stable/13 and stable/14. (sorry, I didn't read the proposed commit message beforehand..)

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?

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?

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).

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).

Deal! Thanks for your maintenance effort.