Page MenuHomeFreeBSD

Add basic python atf support
ClosedPublic

Authored by melifaro on Jul 6 2021, 10:18 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Dec 22, 12:46 AM
Unknown Object (File)
Sat, Dec 21, 4:18 PM
Unknown Object (File)
Sat, Dec 21, 4:10 PM
Unknown Object (File)
Sat, Dec 21, 3:02 PM
Unknown Object (File)
Thu, Dec 19, 7:05 AM
Unknown Object (File)
Wed, Dec 4, 9:51 AM
Unknown Object (File)
Sun, Dec 1, 3:08 PM
Unknown Object (File)
Sat, Nov 30, 3:43 PM

Details

Summary

This diff adds basic framework to write pure python atf-compatible tests.

Problem statement

OS exposes large hundreds of interfaces to interact with. Covering these interfaces with automated tests simplifies developer work and allows them to move fast.
Ability to write the tests cheaply is extremely important - sometimes just a tiny bit results to a written (or non-written) test.

Despite the fact that naive interface is C, writing tests in C may be a bit cumbersome. Setup/teardown wrappers and state introspection are a bit hard to implement in C. Shell support provide pretty good high-level interface, but is limited by the abilities of existing binaries - it is not possible to test specific system call invocation.
Python fills the gap between the two, providing great high-level interfaces and the ability to interact with C code. There are already some tests or test parts, that are written in python. However, without the native support of atf, test framework adopted by FreeBSD, each python test requires a shell wrapper, which is inconvenient.

This diff is a result of considering following intents:

  1. Adding tests should be easier (or at least the same level of complexity) as adding C or shell test
  2. Runner integration with shouldn't restrict framework abilities (think of test parametrisation, for example)
  3. Solution should keep atf/kyua interaction surface as tiny as possible to minimise the migration costs if other runner/test suite is considered
  4. Should allow to easily use/extend common FreeBSD-specific test helpers

This change implements basic atf support, along with some network-related helpers, allowing to easily build test cases.

Implementation

There are a number of implementation approaches that were considered. In order to run python test one needs to (1) pick a testing framework and (2) determine what is the interface to the current test runner, kyua. The former is important, as the framework is what developer actually interacts with. Having convenient functions and decorators simplifies writing tests. Rich introspection interfaces simplify troubleshooting. The latter is important as well - people and automation rely on kyua ability to store and show test stderr/stdout logs, along with the execution results.
Let's start with the framework first. Generally, python testing ecosystem is dominated by 3 popular testing frameworks - built-in unittest, pytest and nose2. The primary unitest benefit is that it exists out-of-the box in stock python. It has less features, leaving pytest and nose as the candidates. Pytest is _really_ popular - for example, its plugin list contains nearly 1k plugins. Its architecture is pretty flexible, allowing to implement the desired support as a module, leaving pytest as the framework of choice.
Current FreeBSD testing framework, kyua, is language-agnostic, which means it executes the tests as separate programs with pre-defined interface. Kyua engine support two such interfaces: TAP and its own, ATF, defined in atf-test-program(4) and atf-test-case(4). TAP is a well-know interface, supported by pytest. However, it interface is extremely simple - there are no test names, debug is limited to one line. It is not possible to pass any test requirements (require.user or timeout). Additionally, implementing "cleanup" logic would also be hard. Cumulatively, it lead to the evaluation and then selection of kyua proprietary protocol, ATF.

Glue all this together.
ATF three key things that are relevant to the design: (1) test listing is a separate test binary run with a special keyword. Kyua expects all test to be returned in a simple text format with key/value pairs. (2) kyua doesn't enforce any structure on stdout/stderr, it gather test result from the filename passed to the test binary. (3) Runner execute test cleanup procedure as a separate binary run, appending :cleanup postfix to the test name.

Implementation consists of the pytest plugin implementing ATF format and a simple C wrapper, which reorders the provided arguments from ATF format to the format understandable by pytest. Each test has this wrapper specified after the shebang. When kyua executes the test, wrapper calls pytest, which loads atf plugin, does the work and returns the result. Additionally, a separate "package", /usr/tests/atf_python was added to collect code that may be useful across different tests. It would be easier to illustrate with examples of listing/running the test to demonstrate all moving parts.

Listing the tests
kyua list -k /path/to/Kyuafile test_file.py calls /path/to/test_file.py -l.
First line of the file is filled by the code in atf_test.mk and looks like the following:
#!/usr/tests/atf_pytest_wrapper -S /usr/tests. Wrapper gets executed with '/usr/tests/atf_pytest_wrapper' '-S /usr/tests' '/path/to/test_file.py' '-l'
Wrapper adds -S /usr/tests to the PYTHONPATH so the tests can reference the aforementioned atf_python package. It then translates the arguments to pytest & plugin lingua:
pytest -p no:cacheprovider -s --atf --co /path/to/test_file.py. Pytest runs, looks into the /path and finds contest.py in /usr/tests, loading the plugin.
Then, pytest does the usual discovery (function names and classes, starting with test_ or Test prefixes).
For the listing, plugin simply suppresses default output (by removing "terminalreporter" plugin) and prints the list of plugins with their metadata. Metadata is gathered from the user-provided marks (@pytest.mark.require_user("root")).

Running the test
kyua debug -k /path/to/Kyuafile test_file.py test_test1 calls /path/to/test_file.py -r /tmp/status_file -s /test/path test_test1.
Wrapper gets executed with '/usr/tests/atf_pytest_wrapper' '-S /usr/tests' '/path/to/test_file.py' '-r' '/tmp/status_file' '-s' '/test/path' 'test_test1'.
Wrapper adds -S /usr/tests to the PYTHONPATH so the tests can reference the aforementioned atf_python package. It also constructs pytest nodeid from the test path and name so only this exact matching test is run.

The resulting rearrangement: pytest -p no:cacheprovider -s --atf --atf-source-dir /test/path --atf-file /tmp/status_file /path/to/test_file.py::test_test1.
Similar to the listing stage, pytest loads the plugin, performes the collection stage and then runs the test. ATF plugin attaches the report hook and translated pytest outcomes to the atf ones. At the terminal phase, plugin writes the test outcome to the supplied file and terminates.

Running cleanup
Cleanup is a biggest "impedance mismatch" between pytest and kyua. The former assumes python-only and really low-side-effect tests, thus promoting running the test cleanups in the same function as the test, after yield. It's also possible to write "teardown" method for the class if the test is class-based. The latter wants to execute cleanup via a separate binary call. This poses some complications with tests discovery.
For example, should the cleanups be seen as a separate tests? It turned out to be a bit problematic to generate the cleanup tests with the parametrised functions. Instead, current implementation patches runtest and setup/teardown hooks of the relevant test:

Cleanup part of kyua debug -k /path/to/Kyuafile test_file.py test_test1 calls /path/to/test_file.py -r /tmp/status_file -s /test/path test_test1:cleanup.
Wrapper notices ":cleanup" test postfix and constructs the following line:
pytest -p no:cacheprovider -s --atf --atf-source-dir /test/path --atf-cleanup --atf-file /tmp/status_file /path/to/test_file.py::test_test1. This is effectively the same as in the running the test, with the --atf-cleanup added. ATF plugin hooks at pytest_collection_modifyitems() and patches the tests so the run the cleanup procedure. The rest part is exactly the same.

What works and what doesn't

Works

  • listing test, running tests, running cleanups
  • Test result mapping (with the side remark that non-strict XFAIL maps to failed, as there is no such state in atf)
  • Test metadata (implemented as marks, like @pytest.mark.require_user("root")) except X-‘NAME’ & require.diskspace

Doesn't

    • Opaque metadata passing via X-Name properties. Require some fixtures to write
  • -s srcdir parameter passed by the runner is ignored.
  • No atf-c-api(3) or similar - relying on pytest framework & existing python libraries
  • No support for atf_tc_<get|has>_config_var() & atf_tc_set_md_var(). Can be probably implemented with env variables & autoload fixtures
    1. Open questions
  • Should the plugin exists as pytest-atf package? It'd love so, but I'm not sure on what's the best way to enforce usage of this plugin when installing tests & provide meaningful error when it doesn't exist.
Test Plan
19:26 [1] m@devel2 kyua list -k /usr/tests/sys/net/routing/Kyuafile test_pytest.py
test_pytest.py:TestClass::test_first
test_pytest.py:TestClass::test_second
test_pytest.py:test_test0_param_check
test_pytest.py:test_test1[a-a]
test_pytest.py:test_test1[bb-bb]
test_pytest.py:test_test2
test_pytest.py:test_test2_cleanup
test_pytest.py:test_test3
test_pytest.py:test_test31
test_pytest.py:test_test4
test_pytest.py:test_test5_marked_xfail_xfailed
test_pytest.py:test_test6_marked_xfail_passed
test_pytest.py:test_test7_marked_xfail_strict_xfailed
test_pytest.py:test_test8_marked_xfail_strict_xfailed
test_pytest.py:test_test9_call_failed

19:26 [1] m@devel2 s kyua test -k /usr/tests/sys/net/routing/Kyuafile test_pytest.py
test_pytest.py:TestClass::test_first  ->  passed  [0.190s]
test_pytest.py:TestClass::test_second  ->  passed  [0.194s]
test_pytest.py:test_test0_param_check  ->  passed  [0.191s]
test_pytest.py:test_test1[a-a]  ->  passed  [0.190s]
test_pytest.py:test_test1[bb-bb]  ->  passed  [0.190s]
test_pytest.py:test_test2  ->  failed: /usr/tests/sys/net/routing/test_pytest.py:29: NameError  [0.203s]
test_pytest.py:test_test2_cleanup  ->  passed  [0.190s]
test_pytest.py:test_test3  ->  skipped: unable to test  [0.189s]
test_pytest.py:test_test31  ->  skipped: unconditional skip  [0.189s]
test_pytest.py:test_test4  ->  skipped: because I can  [0.189s]
test_pytest.py:test_test5_marked_xfail_xfailed  ->  skipped: /usr/tests/sys/net/routing/test_pytest.py:54: XFailed  [0.205s]
test_pytest.py:test_test6_marked_xfail_passed  ->  passed  [0.189s]
test_pytest.py:test_test7_marked_xfail_strict_xfailed  ->  skipped: /usr/tests/sys/net/routing/test_pytest.py:64: XFailed  [0.202s]
test_pytest.py:test_test8_marked_xfail_strict_xfailed  ->  failed: [XPASS(strict)]   [0.189s]
test_pytest.py:test_test9_call_failed  ->  failed: /usr/tests/sys/net/routing/test_pytest.py:74: AssertionError  [0.204s]

Debugging failed test:

19:27 [1] m@devel2 s kyua debug -k /usr/tests/sys/net/routing/Kyuafile test_pytest.py:test_test2
============================= test session starts ==============================
platform freebsd14 -- Python 3.8.13, pytest-7.1.2, pluggy-0.13.1
rootdir: /usr/tests/sys/net/routing
collected 1 item

../../../../usr/tests/sys/net/routing/test_pytest.py TEST2 -> STDOUT MESSAGE
F

=================================== FAILURES ===================================
__________________________________ test_test2 __________________________________

    @pytest.mark.require_user("root")
    def test_test2():
        '''Description 2'''
        print("TEST2 -> STDOUT MESSAGE")
>       print("TEST2 -> STDERR MESSAGE", file=sys.stderr)
E       NameError: name 'sys' is not defined

/usr/tests/sys/net/routing/test_pytest.py:29: NameError
=========================== short test summary info ============================
FAILED ../../../../usr/tests/sys/net/routing/test_pytest.py::test_test2 - Nam...
============================== 1 failed in 0.02s ===============================
test_pytest.py:test_test2  ->  failed: /usr/tests/sys/net/routing/test_pytest.py:29: NameError

Real routing tests with side-effects (VNET&interface creation)

19:29 [1] m@devel2 s kyua test -k /usr/tests/sys/net/routing/Kyuafile test_rtsock_multipath.py
test_rtsock_multipath.py:TestAddRouteWithRta::test_add_route_with_rta  ->  passed  [0.287s]
test_rtsock_multipath.py:TestRtmGetv4ExactSuccess::test_rtm_get_v4_exact_success  ->  passed  [0.279s]


19:30 [1] m@devel2 s kyua debug -k /usr/tests/sys/net/routing/Kyuafile test_rtsock_multipath.py:TestAddRouteWithRta::test_add_route_with_rta
============================= test session starts ==============================
platform freebsd14 -- Python 3.8.13, pytest-7.1.2, pluggy-0.13.1
rootdir: /usr/tests/sys/net/routing
collected 1 item

../../../../usr/tests/sys/net/routing/test_rtsock_multipath.py run: '/sbin/ifconfig epair create'
run: '/usr/sbin/jail -i -c name=jail_test_add_route_with_rta persist vnet vnet.interface=epair0a'
run: 'jexec jail_test_add_route_with_rta /sbin/ifconfig -l'
run: '/sbin/ifconfig epair0a up'
run: '/sbin/ifconfig epair0a inet 192.0.2.1/24'
vvvvvvvv OUT vvvvvvvv
RTM_GET: len 184, pid: 0, seq 1, errno 0, flags: <>
Addrs: <RTA_DST,RTA_NETMASK>
 RTA_DST: 192.0.2.0
 RTA_NETMASK: 255.255.255.0
write(184) -> 'Rtsock' object has no attribute 'assertEqual'
vvvvvvvv  IN vvvvvvvv
RTM_GET: len 200, pid: 65315, seq 1, errno 0, flags: <RTF_UP,RTF_DONE,RTF_PINNED>
Addrs: <RTA_DST,RTA_GATEWAY,RTA_NETMASK>
 RTA_DST: 192.0.2.0
 RTA_GATEWAY: link#3
 RTA_NETMASK: 255.255.255.0
.

============================== 1 passed in 0.08s ===============================
============================= test session starts ==============================
platform freebsd14 -- Python 3.8.13, pytest-7.1.2, pluggy-0.13.1
rootdir: /usr/tests/sys/net/routing
collected 1 item

../../../../usr/tests/sys/net/routing/test_rtsock_multipath.py ==== vnet cleanup ===
run: '/usr/sbin/jail -r jail_test_add_route_with_rta'
run: '/sbin/ifconfig epair0a destroy'
run: '/sbin/ifconfig epair0b destroy'
.

============================== 1 passed in 0.18s ===============================
ifconfig: interface epair0a does not exist
test_rtsock_multipath.py:TestAddRouteWithRta::test_add_route_with_rta  ->  passed

Diff Detail

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

Event Timeline

melifaro added reviewers: tests, kp, lwhsu.

That seems like a good idea for certain types of tests.

tests/sys/Makefile
34 ↗(On Diff #91913)

That looks wrong.

tests/sys/common/vnet.py
30 ↗(On Diff #91913)

That's not always the case. I think this needs to look for the LOOPBACK flag instead.

57 ↗(On Diff #91913)

Someday we'll have Python bindings for libifconfig and we won't have to run_cmd().

I'd love it if you did that for this, but it's a bit unreasonable to ask for that to be done as a prerequisite for this work ;)

195 ↗(On Diff #91913)

I think that bug is fixed by e133271fc1b5e552db6001640891dd8a47371eeb

203 ↗(On Diff #91913)

Don't we already have that function in VnetInterface?

How close are the Python tests to pure ATF sh or c tests? Does it do all of the test-case isolation stuff, like running each test case in its own process in its own temporary directory? Is there a way to skip tests or mark them as expected failures?

How close are the Python tests to pure ATF sh or c tests? Does it do all of the test-case isolation stuff, like running each test case in its own process in its own temporary directory? Is there a way to skip tests or mark them as expected failures?

atf mostly targets the interface that tests should provide to the execution engine, and it is pretty simple: 1) parameters to list/run tests, 2) output file to pass test result and 3) reading test argument from params.

Test isolation is done by the execution engine - Kyua. In the current code, I missed the ability to skip tests, I'll add it in the next iteration - it's trivial.

This, seems like a bit of a misnomer. ATF integration isn’t done: instead unittest is being executed and faking metadata output to sort of act like ATF, but not completely. I would clarify this fact in the commit to avoid overselling what’s being provided.

Are you sure you’ve faked all of the potential metadata permutations?

How does this function when a test case terminates abnormally? Have you tested this case out? What about if listing the test cases fail?

I see opportunities for improvement in this python code (I’ve been doing way too much of this recently for work, which is part of the reason why I haven’t contributed to FreeBSD in ages). Some general best practices that I would apply are:

  • run black.
  • run flake8 (or pylint, maybe; flake8 gives higher signal in my experience).
  • run mypy to confirm that there aren’t any issues with the type hints added, since they still aren’t required/evaluated by the interpreter.
  • run reorder-python-imports.
  • Hardcoding the path to python3 is not desired: someone might want to specify a different path.

—- the shebang should be omitted if I’m not mistaken? It’s been a while since I’ve stared at this code. If not, it would be good to parametrize out the path to the interpreter and/or use ‘/usr/bin/env python3’, etc.

While here: have you investigated pytest? It’s far more extensible and pluggable than unittest.

In D31084#700128, @ngie wrote:

While here: have you investigated pytest? It’s far more extensible and pluggable than unittest.

In general I agree that using a well supported unit test framework seems like it would be better. However, that might require adjusting kyua to parse the output of those tests.
It would be great if we had a simpler test runner than kyua, but changing that would require some significant work.
Maybe another option would be to somehow adjust/parse pytest output? Or we could add support for reading JUnit XML output to kyua.

In D31084#700128, @ngie wrote:

While here: have you investigated pytest? It’s far more extensible and pluggable than unittest.

In general I agree that using a well supported unit test framework seems like it would be better. However, that might require adjusting kyua to parse the output of those tests.
It would be great if we had a simpler test runner than kyua, but changing that would require some significant work.
Maybe another option would be to somehow adjust/parse pytest output? Or we could add support for reading JUnit XML output to kyua.

Seems like there is already an existing TAP output plugin for pytest https://github.com/python-tap/pytest-tap, maybe that could be used for python tests (kyua can already parse TAP output).

In D31084#700128, @ngie wrote:

While here: have you investigated pytest? It’s far more extensible and pluggable than unittest.

In general I agree that using a well supported unit test framework seems like it would be better. However, that might require adjusting kyua to parse the output of those tests.
It would be great if we had a simpler test runner than kyua, but changing that would require some significant work.
Maybe another option would be to somehow adjust/parse pytest output? Or we could add support for reading JUnit XML output to kyua.

Seems like there is already an existing TAP output plugin for pytest https://github.com/python-tap/pytest-tap, maybe that could be used for python tests (kyua can already parse TAP output).

This is a helpful segue.

This plugin is one of many examples of how pytest is frankly superior to raw unittest.

Using that plugin would allow you to more effectively mock TAP, as opposed to the ATF output format. The only real downside as others have discovered is that fine-grained execution of TAP is much more clunky, but it matters less with dynamic output as long as the plugin matches enough of the TAP protocol to interface with kyua.

The benefits of using pytest are numerous, including being able to specify a rich set of fixtures for setup, teardown, parametrize values in a more expressive/idiomatic way (you can sort of do it with unittest, but there’s more boilerplate), etc.

I highly recommend looking at the TAP reporter plugin and pytest instead of investing in a direction that the majority of the python community isn’t going in.

In D31084#700127, @ngie wrote:

This, seems like a bit of a misnomer. ATF integration isn’t done: instead unittest is being executed and faking metadata output to sort of act like ATF, but not completely. I would clarify this fact in the commit to avoid overselling what’s being provided.

Thanks for the feedback!
Could you clarify a bit what do you mean by "fake"? Also, I'm not sure what do you mean when saying "instead unittest". ATF per se is just collection libraries that provide a certain i/o interface to the test program, basic assert wrappers and some util function around getting metadata. This is effectively the implementation of an interface specified in the atf-test-case and (partially) in the atf-test-program.

In D31084#700128, @ngie wrote:

While here: have you investigated pytest? It’s far more extensible and pluggable than unittest.

I don't have any strong opinion on that. If we're okay with requiring pytest, it can be pretty easily converted to it instead.

In D31084#700140, @ngie wrote:
In D31084#700128, @ngie wrote:

While here: have you investigated pytest? It’s far more extensible and pluggable than unittest.

In general I agree that using a well supported unit test framework seems like it would be better. However, that might require adjusting kyua to parse the output of those tests.
It would be great if we had a simpler test runner than kyua, but changing that would require some significant work.
Maybe another option would be to somehow adjust/parse pytest output? Or we could add support for reading JUnit XML output to kyua.

Seems like there is already an existing TAP output plugin for pytest https://github.com/python-tap/pytest-tap, maybe that could be used for python tests (kyua can already parse TAP output).

This is a helpful segue.

This plugin is one of many examples of how pytest is frankly superior to raw unittest.

Using that plugin would allow you to more effectively mock TAP, as opposed to the ATF output format. The only real downside as others have discovered is that fine-grained execution of TAP is much more clunky, but it matters less with dynamic output as long as the plugin matches enough of the TAP protocol to interface with kyua.

For my use case (integration tests around routing control/data plane), the ability to run tests individually and the ability to output detailed debug information are the primary concerns.
When I looked at the tap interface during the evaluation of the possible solutions, I noted that there are problems with both of them. That's why I decided to go with atf support, which will provide unified experience across c/sh tests we use.

The benefits of using pytest are numerous, including being able to specify a rich set of fixtures for setup, teardown, parametrize values in a more expressive/idiomatic way (you can sort of do it with unittest, but there’s more boilerplate), etc.

I highly recommend looking at the TAP reporter plugin and pytest instead of investing in a direction that the majority of the python community isn’t going in.

melifaro retitled this revision from Add python atf support. Add example rtsock tests. to Add basic python atf support.Jun 15 2022, 7:34 PM
melifaro edited the summary of this revision. (Show Details)
melifaro edited the test plan for this revision. (Show Details)
melifaro edited the summary of this revision. (Show Details)

Update the implementation to work on pytest.

ngie requested changes to this revision.Jun 16 2022, 3:23 AM

Please run black on all python files.

share/mk/atf.test.mk
17

How do you specify which version of python to use?

tests/atf_python/atf_pytest.py
3–6

Is intended to work with 3.6+ instead of supporting (soon to be) 3.9+ out of the box? 3.9+ supports native type hints through the generic data types.

80

Any is a bit loose.
Could you please create a simple container object to handle this instead?

176

DRY potential: this statement is called 3 times the same way.

203

This is a much better idiom. It avoids creating a list by walking self._tests_state_map, constructing a temporary list and grabbing the 0th element, by just using an iterator object to grab the first element (and relying on dict.keys() to be iterated over by default with dict.__iter__()).
Doesn't matter so much in this case given the dataset size, but in general, it turns an O(n) algorithm into an O(1) algorithm.

209

line is always guaranteed to be a UTF-8 (or whatever the default encoding is) str?

tests/atf_python/sys/net/rtsock.py
29–71

This should really use CFFI.

tests/freebsd_test_suite/atf_pytest_wrapper.c
1 ↗(On Diff #106960)

*shudders because this is written in C, not modern C++*

tests/sys/net/routing/test_pytest.py
68 ↗(On Diff #106960)

There are a ton of useless pass statements: pylint and other style checkers would complain about them being there.

As long as a docstring comment is present, pass isn't needed.

This revision now requires changes to proceed.Jun 16 2022, 3:23 AM

Thank you for the productive feedback!

In D31084#805155, @ngie wrote:

Please run black on all python files.

Did that - was noop aside from adding spaces to [:] constructions which flake8 is not happy with :-)
Re-runned after addressing the changes.

melifaro marked 2 inline comments as done.

Remove unnecessary "pass" statements.

melifaro added inline comments.
share/mk/atf.test.mk
17

I rely on the pytest to determine one. wrapper runs pytest from the current $PATH. Either stock or venv-baased one can be used.
Do you think it's not flexible enough?

tests/atf_python/atf_pytest.py
3–6

I understand and value importance of typing - it indeed greatly helps to avoid weird failures in larger projects.
Here the module surface is relatively small (and won't probably change much) so my preference would be to not be too restrictive and allow 3.6+ (or actually 3.7 which current pytest is based on).

80

Does it look better now?

209

Well, it should be normally the case, but I'm not sure if one need to bother to address weird corner cases - ATF runner should address that.

tests/atf_python/sys/net/rtsock.py
29–71

Would love to do that, but I don't see any easy way to do it.

In in-line usage you can't import headers (b/c of #ifdef guard). For API mode you need to manually address all dependency chain (it's non-empty). You may also need to handle .py in Makefiles. Finally, you need to properly address fetching the namespace for defines (I don't see any way aside from directly fetching ffi._parser._int_constants). Properly integrating the stuff requires non-trivial amount of time which I don't currently have. This already depth 2 of the recursion stack :-)

tests/sys/net/routing/test_pytest.py
68 ↗(On Diff #106960)

I'm not sure if I want to commit it as is (it's actually module tests) but I've removed the redunant pass keywords.

melifaro added inline comments.
tests/freebsd_test_suite/atf_pytest_wrapper.c
1 ↗(On Diff #106960)

It started as a simple 10-lines program but slowly mutated to what it's now :-)
Does the current version look better now?

Thank you for the productive feedback!

In D31084#805155, @ngie wrote:

Please run black on all python files.

Did that - was noop aside from adding spaces to [:] constructions which flake8 is not happy with :-)
Re-runned after addressing the changes.

Yeah... the black defaults don't really play well with flake8's/pylint's defaults: https://black.readthedocs.io/en/stable/guides/using_black_with_other_tools.html#flake8 .

Also folks, aside from the (extremely valuable) detailed technical feedback on the implementation details I'd really love to get general feedback.
Is this the direction we want to go? Does the implementation (generally) look sane?
If the answer is yes, for at least the first question, I'd like to remind that there is an open question on whether we want the pytest-atf binding be embedded in the tree, as it currently is, or exist as a separate package (python-atf or whatever).

Is this the direction we want to go? Does the implementation (generally) look sane?

It does to me, but I'll defer to people like ngie, with more python experience.

I'd like to remind that there is an open question on whether we want the pytest-atf binding be embedded in the tree, as it currently is, or exist as a separate package (python-atf or whatever).

We already have python code in the tree for tests, so it makes sense to me to just have this in the tree as well. We just skip these tests if python is not installed, and it'll be easier to evolve the interface between kyua and python-atf if it's all in one tree.

Add actual multipath tests. Improve python helpers.

I’m going to commit it tomorrow, june 25 unless I get any objections

This revision was not accepted when it landed; it landed in state Needs Review.Jun 25 2022, 7:32 PM
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.