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)
Tue, May 28, 5:13 AM
Unknown Object (File)
Apr 27 2024, 5:54 PM
Unknown Object (File)
Apr 27 2024, 5:52 PM
Unknown Object (File)
Apr 27 2024, 5:52 PM
Unknown Object (File)
Apr 27 2024, 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
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
rS FreeBSD src repository - subversion
Lint
Lint Errors
SeverityLocationCodeMessage
Errortests/atf_python/atf_pytest.py:45E731PEP8 E731
Errortests/atf_python/atf_pytest.py:46E701PEP8 E701
Errortests/atf_python/atf_pytest.py:81E701PEP8 E701
Errortests/atf_python/atf_pytest.py:82E701PEP8 E701
Errortests/atf_python/atf_pytest.py:85E701PEP8 E701
Errortests/atf_python/atf_pytest.py:224E501PEP8 E501
Errortests/conftest.py:60E501PEP8 E501
Errortests/conftest.py:64E501PEP8 E501
Errortests/conftest.py:99E501PEP8 E501
Unit
No Test Coverage
Build Status
Buildable 46146
Build 43035: arc lint + arc unit

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

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

'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.