Page MenuHomeFreeBSD

testing: pass ATF vars to pytest via env instead of arguments.
ClosedPublic

Authored by melifaro on Jun 28 2022, 10:58 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Apr 27, 5:54 PM
Unknown Object (File)
Sat, Apr 27, 5:52 PM
Unknown Object (File)
Sat, Apr 27, 5:52 PM
Unknown Object (File)
Sat, Apr 27, 4:29 PM
Unknown Object (File)
Apr 25 2024, 4:23 AM
Unknown Object (File)
Apr 25 2024, 4:23 AM
Unknown Object (File)
Apr 22 2024, 4:51 AM
Unknown Object (File)
Apr 22 2024, 4:38 AM
Subscribers

Details

Summary

This change is a continuation of 9c42645a1e4d workaround.
Apparently pytest argument parser is not happy when parsing values
with spaces or just more than one --atf-var argument.
Switch wrapper to send these kv pairs as env variables. Specifically,
use _ATF_VAR_key=value format to distinguish from the other vars.

Add the atf_vars fixture returning all passed kv pairs as a dict.

MFC after: 2 weeks

Test Plan

Test:

def test_params(self, atf_vars):
    print(os.environ)
    print(atf_vars)
    print(atf_vars.get("fibs"))

Execution (test suit vm):

kyua debug -k /usr/tests/sys/net/routing/Kyuafile test_rtsock_multipath.py:TestRtmMultipath::test_params
epair0a: Ethernet address: 02:4a:42:75:48:0a
epair0b: Ethernet address: 02:4a:42:75:48:0b
epair0a: link state changed to UP
epair0b: link state changed to UP
epair0a: link state changed to DOWN
epair0b: link state changed to DOWN
============================= test session starts ==============================
platform freebsd14 -- Python 3.8.13, pytest-7.1.2, pluggy-1.0.0
rootdir: /usr/tests/sys/net/routing
collected 1 item

../../../../usr/tests/sys/net/routing/test_rtsock_multipath.py
environ({'LC_CTYPE': 'C.UTF-8', '_ATF_VAR_unprivileged-user': 'tests', '_ATF_VAR_fibs': '1 2', '_ATF_VAR_disks': '/dev/vtbd2 /dev/vtbd3 /dev/vtbd4 /dev/vtbd5 /dev/vtbd6', '_ATF_VAR_ci': 'true', '_ATF_VAR_cam_test_device': '/dev/ada0', '_ATF_VAR_allow_sysctl_side_effects': '1', 'PYTHONPATH': '/usr/tests', '__RUNNING_INSIDE_ATF_RUN': 'internal-yes-value', 'TZ': 'UTC', 'TMPDIR': '/tmp/kyua.HOX4TI/2/work', 'HOME': '/tmp/kyua.HOX4TI/2/work', 'LOGNAME': 'root', 'PAGER': 'less', 'MAIL': '/var/mail/root', 'PATH': '/sbin:/bin:/usr/sbin:/usr/bin:/usr/local/sbin:/usr/local/bin:/root/bin', 'ENV': '/root/.shrc', 'PWD': '/root', 'TERM': 'vt100', 'USER': 'root', 'SHELL': '/bin/sh', 'MM_CHARSET': 'UTF-8', 'BLOCKSIZE': 'K', 'PYTEST_CURRENT_TEST': 'test_rtsock_multipath.py::TestRtmMultipath::test_params (setup)'})

run: '/sbin/ifconfig epair create'
run: '/usr/sbin/jail -i -c name=jail_test_params persist vnet vnet.interface=epair0a'
run: 'jexec jail_test_params /sbin/ifconfig -l'
{'unprivileged-user': 'tests', 'fibs': '1 2', 'disks': '/dev/vtbd2 /dev/vtbd3 /dev/vtbd4 /dev/vtbd5 /dev/vtbd6', 'ci': 'true', 'cam_test_device': '/dev/ada0', 'allow_sysctl_side_effects': '1'}
1 2
.

============================== 1 passed in 1.09s ===============================

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

melifaro added reviewers: tests, kp, lwhsu.

I feel it's a bit overkill to use envvar, but let's fix the tests first.

libexec/atf/atf-pytest-wrapper/atf_pytest_wrapper.cpp
107–114

mostly a question, are those brackets needed?

This revision is now accepted and ready to land.Jun 28 2022, 11:41 AM

I feel it's a bit overkill to use envvar, but let's fix the tests first.

Ty for reviewing!

I'm not particularly happy with it either - it would be cleaner and simpler to pass all the metadata in the same way.
Unfortunately, pytest version of argparser behaves weirdly and breaks randomly. Even if I spend enough time and isolate&fix an issue with the parser, it will take months to propagate to the next release & our packages.
Still, this solution works, is relatively simple and doesn't add too much overhead, especially considering the whole atf paradigm of running every test and test cleanup as a separate process.
Personally I don't see a good value vs time spent on fixing it balance afterwards (except the case when someone fixes the parser stuff). Thoughts?

libexec/atf/atf-pytest-wrapper/atf_pytest_wrapper.cpp
107–114

'case X' internally is just a jump within a scope, so all of the switch statements {} share the same scope.
C++ prohibits jumping w/o variable initialisation (and even 'std::string kv' has default initialiser). Thus, we use traditional c-style solution e.g. creation of a new scope.

This revision was landed with ongoing or failed builds.Jun 28 2022, 12:20 PM
This revision was automatically updated to reflect the committed changes.