Date: Tue, 8 Oct 2019 23:22:48 -0700 From: "Enji Cooper (yaneurabeya)" <yaneurabeya@gmail.com> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Eric van Gyzen <vangyzen@FreeBSD.org>, src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: Re: svn commit: r353305 - head/tests/sys/kern Message-ID: <AF44B7CF-FC21-4C8D-B8CE-990E704416A5@gmail.com> In-Reply-To: <20191008150438.GE44691@kib.kiev.ua> References: <201910081343.x98Dh5bW006905@repo.freebsd.org> <20191008150438.GE44691@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
> On Oct 8, 2019, at 08:04, Konstantin Belousov <kostikbel@gmail.com> wrote: > > On Tue, Oct 08, 2019 at 01:43:05PM +0000, Eric van Gyzen wrote: >> Author: vangyzen >> Date: Tue Oct 8 13:43:05 2019 >> New Revision: 353305 >> URL: https://svnweb.freebsd.org/changeset/base/353305 >> >> Log: >> Fix problems in the kern_maxfiles__increase test >> >> ATF functions such as ATF_REQUIRE do not work correctly in child processes. >> Use plain C functions to report errors instead. > There are much more tests that fork and use ATF_ in children. > Look e.g. at most ptrace(2) tests. I beg to disagree: 86 /* 87 * A variant of ATF_REQUIRE that is suitable for use in child 88 * processes. This only works if the parent process is tripped up by 89 * the early exit and fails some requirement itself. 90 */ 91 #define CHILD_REQUIRE(exp) do { \ 92 if (!(exp)) \ 93 child_fail_require(__FILE__, __LINE__, \ 94 #exp " not met"); \ 95 } while (0) The issue, in this particular case, and the item that evangyzen was fixing, was the fact that failures in children can result in very confusing failures from a parent level. In particular, ATF_CHECK does not trickle up to parents and ATF_REQUIRE* gets thrown up to parents as abort()’ed processes. The best way to handle this within the atf-c/atf-c++ framework (with less boilerplate) is to use these APIs: atf_utils_fork(..)/atf_utils_wait(..). You will still need to use `_exit` (instead of exit(..)/assert(..)/ATF_CHECK(..)/ATF_REQUIRE(..), but it’s a lot less boilerplate than writing it yourself. Again, this is why I was driving towards GoogleTest. Despite the fact that it’s a C++ framework, there’s a lot less confusing boilerplate involved and context, since things are executed in relatively the same context, i.e., setup -> test -> teardown, and they’re easier to follow. The best way to move forward usability-wise with this (I think) is to probably alias the ATF_* macros to something more sensible when forking processes. However, given the amount of complaints I’ve heard about ATF, I think it’s best not to build upon an unstable foundation, but instead encourage the use of something more widely-accepted across the open source community/straightforward use wise. In this case, googletest. Thanks, -Enji
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AF44B7CF-FC21-4C8D-B8CE-990E704416A5>