On Mon, 2025-02-10 at 13:51 +0000, Mykyta Yatsenko wrote:
[...]
> +void test_veristat_set_global_vars_succeeds(void)
> +{
test_progs based tests are usually organized as a hierarchy of tests and sub-tests.
E.g. take a look at tools/testing/selftests/bpf/prog_tests/ksyms_btf.c:
- it defines an entry point test_ksyms_btf;
- and a bunch of sub-tests declared as static void functions,
called from entry point;
- test__start_subtest() function is used to check if sub-test has to
be executed.
> + char command[512];
> + struct fixture *fix = init_fixture();
> +
> + snprintf(command, sizeof(command),
> + "./veristat set_global_vars.bpf.o"\
> + " -G \"var_s64 = 0xf000000000000001\" "\
> + " -G \"var_u64 = 0xfedcba9876543210\" "\
> + " -G \"var_s32 = -0x80000000\" "\
> + " -G \"var_u32 = 0x76543210\" "\
> + " -G \"var_s16 = -32768\" "\
> + " -G \"var_u16 = 60652\" "\
> + " -G \"var_s8 = -128\" "\
> + " -G \"var_u8 = 255\" "\
> + " -G \"var_ea = EA2\" "\
> + " -G \"var_eb = EB2\" "\
> + " -G \"var_ec = EC2\" "\
> + " -G \"var_b = 1\" "\
> + "-vl2 > %s", fix->tmpfile);
> + if (!ASSERT_EQ(0, system(command), "command"))
> + goto out;
Nit: there is SYS macro in test_progs.h, it combines
snprintf/system/ASSERT_OK/goto.
> +
> + read(fix->fd, fix->output, fix->sz);
Nit: check error for read() call (same read()/write() in tests below).
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=0xf000000000000001 "),
> + "var_s64 = 0xf000000000000001");
Nit: I'd do these checks as below:
#define __CHECK_STR(str, name) \
if (!ASSERT_HAS_SUBSTR(fix->output, (str), (str))) goto out
__CHECK_STR("_w=0xf000000000000001 ");
...
#undef __CHECK_STR
this way fix->output would be printed if sub-string is not found.
For other tests I suggest using ASSERT_HAS_SUBSTR as well,
as it prints the string where sub-string was looked for.
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=0xfedcba9876543210 "),
> + "var_u64 = 0xfedcba9876543210");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=0x80000000 "), "var_s32 = -0x80000000");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=0x76543210 "), "var_u32 = 0x76543210");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=0x8000 "), "var_s16 = -32768");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=0xecec "), "var_u16 = 60652");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=128 "), "var_s8 = -128");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=255 "), "var_u8 = 255");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=11 "), "var_ea = EA2");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=12 "), "var_eb = EB2");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=13 "), "var_ec = EC2");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=1 "), "var_b = 1");
> +
> +out:
> + teardown_fixture(fix);
> +}
> +
> +void test_veristat_set_global_vars_from_file_succeeds(void)
> +{
> + struct fixture *fix = init_fixture();
> + char command[512];
> + char input_file[80];
> + const char *vars = "var_s16 = -32768\nvar_u16 = 60652";
> + int fd;
> +
> + snprintf(input_file, sizeof(input_file), "/tmp/veristat_input.XXXXXX");
> + fd = mkstemp(input_file);
> + if (!ASSERT_GT(fd, 0, "valid fd"))
Nit: ASSERT_GE.
> + goto out;
> +
> + write(fd, vars, strlen(vars));
> + snprintf(command, sizeof(command),
> + "./veristat set_global_vars.bpf.o -G \"@%s\" -vl2 > %s",
> + input_file, fix->tmpfile);
> +
> + ASSERT_EQ(0, system(command), "command");
> + read(fix->fd, fix->output, fix->sz);
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=0x8000 "), "var_s16 = -32768");
> + ASSERT_NEQ(NULL, strstr(fix->output, "_w=0xecec "), "var_u16 = 60652");
> +
> +out:
> + close(fd);
> + remove(input_file);
> + teardown_fixture(fix);
> +}
[...]
