Return explicit status codes from tests to indicate pass/fail/skip

[Qix-]

This seems like a huge bug surface area - is there a reason why there's an if (argc > 1) return 0;? Why not return an error code? This means that if an argument is mistakenly passed to a test, it'll silently show a false-positive.

[isilence]

You probably forgot to specify a file name, a function or anything to locate this. You may also just send a patch

[Qix-]

That's not what I mean. All of the tests have

int main(int argc, char *argv[]) {
    if (argc > 1) return 0;

    /* test code */

    return 0;
}

This means if you ever pass ./some-test some-arg to a test, then it'll silently report that the test was successful. This is a huge risk for false positives if the testbed were to ever pass an argument to the tests. That's my point. I didn't forget anything.

I'm curious if there's a reason for this check that I'm missing, but I can't see why it would be so in the testbed.

[isilence]

Ok, looks strange indeed, but I haven't had a problem with it though. Maybe @axboe needed that for some reason.

[Qix-]

yeah I'm sure it hasn't caused problems but I was adding a test and noticed it, figured it seemed kind of strange and was a potential cause for false positives down the line.

[axboe]

It's done on purpose - tests that are able to take an argument with run with that argument, tests that don't just exit to not repeat the exact same test. My test setup has a lot of different file and devices set in config.local, and make runtests will run each test on on each device/file listed.

So it's very much done on purpose. One thing that would be nice is have a normalized test return - eg TEST_ERROR, TEST_OK, and TEST_SKIPPED. Then we could return SKIPPED for this case, too.

[Qix-]

@axboe Feel free to re-open. I'll re-title and submit a PR to do exactly what you said in your last sentence as I agree entirely :)

[axboe]

That'd be great, thanks!

[axboe]

Tests do return fail/success/skip now.