Re: [PATCH v3 7/9] rv/tlob: add KUnit tests for the tlob monitor

[Gabriele Monaco <gmonaco@redhat.com>]

On Mon, 2026-06-08 at 00:13 +0800, wen.yang@linux.dev wrote:
> +/*
> + * Valid add lines return -ENOENT (kern_path() finds no such file in
> the test
> + * environment) rather than 0; a non-(-EINVAL) return confirms the
> format was
> + * accepted by the parser.
> + */
> +static void tlob_parse_valid_accepted(struct kunit *test)
> +{
> +	char buf[128];
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tlob_parse_valid); i++) {
> +		strscpy(buf, tlob_parse_valid[i], sizeof(buf));
> +		KUNIT_EXPECT_NE(test,
> tlob_create_or_delete_uprobe(buf), -EINVAL);

Can you perhaps add those uprobes for real from this test case?
Unit tests should not touch the system, you usually do that by: 

1. stubbing the tested function when it starts doing bad things
2. test with dummy data not attaching anything (not applicable here)
3. test a different function not affecting the system

I think the cleanest here is 3. so you could just kunit test
tlob_parse_uprobe_line() and tlob_parse_remove_line().

Alternatively just stub the entirety of tlob_add_uprobe()
tlob_remove_uprobe_by_key() and maybe even check that they're called
when expected and not called on failure (right now you aren't testing
valid removals, probably because that's going to break).

I believe a good unit test should be validating the parsing logic only
/or/ the add/remove logic (but that's hard, you can skip it or even
check in selftests).
Right now your tests are trying to do both, so you don't know if
failures came from the uprobes subsystem or allocation (you shouln't
even get there from the unit test).

Then you can just check for success and not for ! EINVAL , which is
confusing.

Thanks,
Gabriele

> +	}
> +}
> +
> +static void tlob_parse_invalid_rejected(struct kunit *test)
> +{
> +	char buf[128];
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tlob_parse_invalid); i++) {
> +		strscpy(buf, tlob_parse_invalid[i], sizeof(buf));
> +		KUNIT_EXPECT_EQ(test,
> tlob_create_or_delete_uprobe(buf), -EINVAL);
> +	}
> +}
> +
> +static void tlob_parse_out_of_range_rejected(struct kunit *test)
> +{
> +	char buf[128];
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(tlob_parse_out_of_range); i++) {
> +		strscpy(buf, tlob_parse_out_of_range[i],
> sizeof(buf));
> +		KUNIT_EXPECT_EQ(test,
> tlob_create_or_delete_uprobe(buf), -ERANGE);
> +	}
> +}
> +
> +static struct kunit_case tlob_parse_cases[] = {
> +	KUNIT_CASE(tlob_parse_valid_accepted),
> +	KUNIT_CASE(tlob_parse_invalid_rejected),
> +	KUNIT_CASE(tlob_parse_out_of_range_rejected),
> +	{}
> +};
> +
> +static struct kunit_suite tlob_parse_suite = {
> +	.name       = "tlob_parse",
> +	.test_cases = tlob_parse_cases,
> +};
> +
> +kunit_test_suite(tlob_parse_suite);
> +
> +MODULE_DESCRIPTION("KUnit tests for the tlob RV monitor");
> +MODULE_LICENSE("GPL");