search

freebsd / kyua

Port/Package build and test system
https://github.com/freebsd/kyua/wiki
BSD 3-Clause "New" or "Revised" License
149 stars 42 forks source link

atf_tc_get_config_var doesn't work for variables set in kyua.conf #108

Open asomers opened 10 years ago

asomers commented 10 years ago

atf_tc_get_config_var doesn't seem to work 100% correctly for variables set in /usr/local/etc/kyua.conf. I discovered this when investigating issue #107. Some tests erroneously required "unprivileged-user" instead of "unprivileged_user". First of all, they shouldn't have run at all, because "unprivileged-user" was not set in my kyua.conf and they had a 'atf_tc_set_md_var(tc, "require.config", "unprivileged-user");' line in their headers. Secondly, after I fixed the variable name, they all segfault now. Below is a partial stacktrace for one failure.

I haven't found any working examples, in either devel/atf, devel/kyua, or freebsd/head, of atf_tc_get_config_var being used to read a variable from kyua.conf. Aside from run_test.c, it's only used to read srcdir.

#kyua debug run_test:fork_wait__unprivileged_group
Process with PID 75009 dumped core; attempting to gather stack trace
(no debugging symbols found)...
warning: exec file is newer than core file.
Core was generated by `run_test'.
Program terminated with signal 11, Segmentation fault.
#0  0x0000000800839a89 in atf_map_citer_data (citer=<value optimized out>)
    at atf-c/detail/map.c:92
92      return me->m_value;
GDB exited successfully
run_test:fork_wait__unprivileged_group  ->  broken: Premature exit; test case received signal 11 (core dumped)
asomers commented 10 years ago

I understand the issue now. There are five things going on:

1) As I commented in issue #107, ATF programs expect an "unprivileged-user" variable, whereas kyua.conf expects an "unprivileged_user" variable. engine/metadata.cpp does the renaming. So I was trying to read the wrong variable with atf_tc_get_config_var

2) require.configs is verified by Kyua, not by ATF. And it will accept either the hyphen or underscore version of unprivileged_user. This is a bug, because "unprivileged_user" doesn't exist for ATF test programs.

3) Assertions were disabled in libatf. Had they been enabled, then the segfault would've turned into an assertion, and I would've solved the problem more quickly. IMHO, assertions should always be enabled for non-performance critical programs like ATF.

4) atf_tc_get_config_var segfaults (or asserts) if the argument doesn't exist. So the caller is responsible for calling atf_tc_has_config_var() first. IMHO that's undesirable for a public API. I think that a libatf consumer shouldn't be able to trigger an assertion. It would be better to mark the test case as broken with an appropriate error message about the variable being undefined.

5) engine/testers.cpp forwards unprivileged_user and all of the test_suite specific config variables to the test program's command line. However, it looks no other config variable (currently, only architecture and platform fall into this category) get passed along. That violates POLA. I don't see anything in kyua-tester(1) or any of the atf docs that suggests that you should be unable to get those config variables. I think it would be preferable that they be available to the test program.