Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The runner ignores exit statuses after the test is done executing #220

Open
Snaipe opened this issue Oct 9, 2017 · 2 comments
Open

The runner ignores exit statuses after the test is done executing #220

Snaipe opened this issue Oct 9, 2017 · 2 comments

Comments

@Snaipe
Copy link
Owner

Snaipe commented Oct 9, 2017

After the discussion with @cbatten on #204 about address sanitizer, I went on to test my allegations since it's been a few releases since I haven't checked that particular synergy.

It turns out that the introduction of the new client/server code broke it. In particular, this bit reports the test success (or failure), but this happens while the worker is still alive -- and while an exit called in the test is going to trigger the death event, and fail the test, Address Sanitizer and Valgrind on the other hand exit with a non-zero exit status after the main has done executing, which is well after the success or failure of the test has been reported and printed.

The incorrect condition came from the change of semantics in the runner's protocol: before the refactor, the worker process was the test itself. Now, the runner conceptually supports workers that may run more than one test.

I think the best fix for that is to simply restrict the procotol to only accept one test per process, and then handle the reporting after the worker exits. Additional tests can always be run as sub-tests of the process "root" test.

@cbatten
Copy link

cbatten commented Oct 9, 2017

Interesting. Thanks for looking into this! I also noticed this when you mentioned that a valgrind error would cause the test to fail. I tried this:

#include <criterion/criterion.h>
#include <stdlib.h>

Test( leak, leak )
{
  int* ptr = malloc( sizeof(int) );
}

And then ran it like this:

 % valgrind --leak-check=full --trace-children=yes --error-exitcode=1 ./leak

And valgrind reports a memory leak error but the test still passes.

@Snaipe
Copy link
Owner Author

Snaipe commented Oct 9, 2017

Yes, this is the same issue. It used to work before the changes to the i/o layer, as the final exit status was used as a final override on the success failure of the test. Moving the actual test status report after the process exit should restore that behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants