On 7/22/24 2:55 AM, Marco Elver wrote:
On Sat, Jul 20, 2024 at 09:54AM -0700, Kees Cook wrote:
...
I'm more confused now. This is just moving tests further away from what they are testing for no good reason. If there's a directory "foo", then moving things to "tests/foo" is unclear. It's unclear if "tests" is inside parent of "foo" or actually a subdir of "foo". Per the paragraph above, I inferred it's "foo/tests/foo/...", which is horrible. If it's "../tests/foo/..." it's also bad because it's just moving tests further away from what they are testing. And keeping tests close to the source files under test is generally considered good practice, as it avoids the friction required to discover where tests live. Moving tests to "../tests" or "../../*/tests" in the majority of cases is counterproductive. It is more important for people to quickly discover tests nearby and actually run them, vs. having them stashed away somewhere so they don't bother us. While we can apply common sense, all too often someone follows these rules blindly and we end up with a mess.
Here, you've actually made a good argument for "blindly" following the new naming/location conventions: it's easier to find things if a standard naming and location convention is in place. Especially if we document it. Now if only someone would post a patch with such documentation... :) I would add that the "_kunit" part of the name is especially helpful, because (as I mentioned earlier) these tests really are different enough that it's worth calling out. You can run them simply by loading the kernel module. So if I want to quickly run kunit tests, searching for "*_kunit.c" does help with that. thanks, -- John Hubbard NVIDIA
