On Tue, Apr 20, 2021 at 4:23 AM Lorenz Bauer <lmb@xxxxxxxxxxxxxx> wrote:
>
> Hi Andrii,
>
> I was looking at some CORE testcases, and noticed two problems:
>
> * The checks for negative test cases use an incorrect CHECK(false)
> invocation. This means negative test cases don't fail when they
> should.
CHECK() misuse? shocker ;) This is why I'm harping on ASSERT_xxx() vs
CHECK() all the time lately.
> * Some existence tests use incorrect file names, but the test harness
> is unable to detect this. Basically, failure to load due to a failed
> CORE relocation is not distinguished from ENOENT. I found the CHECK
> issue when investigating this problem.
>
> I've written the patch attached below, but there are now 12 failures.
> I don't understand the tests well enough to fix them, maybe you can
> take a look?
Yep, thanks for reporting, I'll take a look.
>
> Best
> Lorenz
>
> ---
> .../selftests/bpf/prog_tests/core_reloc.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> index d94dcead72e6..bd759290347c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> +++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
> @@ -644,12 +644,12 @@ static struct core_reloc_test_case test_cases[] = {
> .output_len = sizeof(struct core_reloc_existence_output),
> },
>
> - FIELD_EXISTS_ERR_CASE(existence__err_int_sz),
> - FIELD_EXISTS_ERR_CASE(existence__err_int_type),
> - FIELD_EXISTS_ERR_CASE(existence__err_int_kind),
> - FIELD_EXISTS_ERR_CASE(existence__err_arr_kind),
> - FIELD_EXISTS_ERR_CASE(existence__err_arr_value_type),
> - FIELD_EXISTS_ERR_CASE(existence__err_struct_type),
> + FIELD_EXISTS_ERR_CASE(existence___err_wrong_int_sz),
> + FIELD_EXISTS_ERR_CASE(existence___err_wrong_int_type),
> + FIELD_EXISTS_ERR_CASE(existence___err_wrong_int_kind),
> + FIELD_EXISTS_ERR_CASE(existence___err_wrong_arr_kind),
> + FIELD_EXISTS_ERR_CASE(existence___err_wrong_arr_value_type),
> + FIELD_EXISTS_ERR_CASE(existence___err_wrong_struct_type),
>
> /* bitfield relocation checks */
> BITFIELDS_CASE(bitfields, {
> @@ -864,7 +864,7 @@ void test_core_reloc(void)
> err = bpf_object__load_xattr(&load_attr);
> if (err) {
> if (!test_case->fails)
> - CHECK(false, "obj_load", "failed to load prog '%s': %d\n", probe_name, err);
> + CHECK(true, "obj_load", "failed to load prog '%s': %d\n", probe_name, err);
> goto cleanup;
> }
>
> @@ -904,7 +904,7 @@ void test_core_reloc(void)
> }
>
> if (test_case->fails) {
> - CHECK(false, "obj_load_fail", "should fail to load prog '%s'\n", probe_name);
> + CHECK(true, "obj_load_fail", "should fail to load prog '%s'\n", probe_name);
> goto cleanup;
> }
>
> --
> 2.27.0
>
