Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Errors - Unit
No Test Coverage - Build Status
Buildable 50784 Build 47675: arc lint + arc unit
Event Timeline
If this is is just a PoC for the discussion in D39420, please ignore. Otherwise, I believe this will need an ObsoleteFiles.inc entry?
The only drawback I can find to python tests is that they run a bit slower (for now [1]), allegedly compensated by reduced development time :-))
# kyua test sys/netinet6/forward6 sys/netinet6/forward6:fwd_ip6_gu_icmp_gw_gu_fast_success -> passed [5.536s] sys/netinet6/forward6:fwd_ip6_gu_icmp_gw_gu_slow_success -> passed [5.296s] sys/netinet6/forward6:fwd_ip6_gu_icmp_gw_ll_fast_success -> passed [5.493s] sys/netinet6/forward6:fwd_ip6_gu_icmp_gw_ll_slow_success -> passed [5.587s] sys/netinet6/forward6:fwd_ip6_gu_icmp_iface_fast_success -> passed [5.056s] sys/netinet6/forward6:fwd_ip6_gu_icmp_iface_slow_success -> passed [4.729s]
# kyua test sys/netinet6/test_ip6_forward.py sys/netinet6/test_ip6_forward.py:TestIP6Forward::test_success[gu-fast] -> passed [7.736s] sys/netinet6/test_ip6_forward.py:TestIP6Forward::test_success[gu-slow] -> passed [7.825s] sys/netinet6/test_ip6_forward.py:TestIP6Forward::test_success[if-fast] -> passed [7.847s] sys/netinet6/test_ip6_forward.py:TestIP6Forward::test_success[if-slow] -> passed [7.861s] sys/netinet6/test_ip6_forward.py:TestIP6Forward::test_success[ll-fast] -> passed [7.927s] sys/netinet6/test_ip6_forward.py:TestIP6Forward::test_success[ll-slow] -> passed [7.841s]
[1]: I'm investigating setup and teardown.
This is neat. It took me some time to understand what's going on, but certainly this framework is better than atf-sh for writing complex or heavily parameterized tests.
Anything you can do to improve python test case runtimes would be appreciated. I run the regression tests in qemu quite frequently (partially because that can be done without any special system privileges) and python tests are very slow there. Parallelization helps somewhat.
| tests/sys/netinet6/test_ip6_forward.py | ||
|---|---|---|
| 15 | To me this needs a comment explaining the high-level idea behind the test. It is not very easy to tell what this is actually doing. | |
| 136 | This is a bit magical. Why not send the command you want the handler to execute, instead of having some apparently hard-coded list of messages? Or have some dictionary mapping message names to actions in the handler, so that it's a bit easier to see what's going on. | |
I'm currently trying to write a carp test based on this example, and ... well, I can sort of see it making sense when I need scapy to parse packets (which is what I want to do in the test I'm writing), but there's a lot of magic here, and it's taken me quite a bit of time already just to get the setup going (a vnet jail with one epair interface with carp configured on it), and it still doesn't do things that are trivial in atf-sh (How do I skip the test if carp.ko is not loaded?).
Having more or less finished that test (D39454) I think my view remains that python is great for tests that do things like use scapy to parse packets, but I don't think they'd be suitable for tests such as pfsync:bulk (and indeed the majority of pf tests) where 99% of the test is setup.
There's a fair criticism of the atf-sh tests that there's a lot of boilerplate, but I find it makes the test more obvious about what's going on. That is, I can read the test as if it were a very simple shell script. Nothing is hidden, and it's clear what setup is being created.
I believe that in some cases, complex templates can be created, for example:
outside
|
+---+---+
fw2 --- fw2
+---+---+
|
insidecan be a class named SimplePFSync that you can inherit in your tests. You would need to understand what SimplePFSync does and returns first (as a newcomer), also it will likely reside in some common file somewhere else (as I understand you prefer to see the setup explicitly in the test), but it should allow you to focus on what's being tested. It is a matter of personal preference, of course.
You can use REQUIRED_MODULES list in the test class. Please see an example here: https://github.com/freebsd/freebsd-src/blob/main/tests/examples/test_examples.py
Do you have any suggestions on the approach to reduce the "understanding" phase? There are some framework examples in tests/examples, but apparently that's not enough. Do you think that, for example, atf-python(7) may help in that regard?
Interesting, that looks quite high. On my VM:
19:43 [1] m@devel2 s kyua test -k /usr/tests/sys/netinet6/Kyuafile test_ip6_forward.py test_ip6_forward.py:TestIP6Forward::test_success[gu-fast] -> passed [1.658s] test_ip6_forward.py:TestIP6Forward::test_success[gu-slow] -> passed [1.599s] test_ip6_forward.py:TestIP6Forward::test_success[if-fast] -> passed [1.571s] test_ip6_forward.py:TestIP6Forward::test_success[if-slow] -> passed [1.688s] test_ip6_forward.py:TestIP6Forward::test_success[ll-fast] -> passed [1.744s] test_ip6_forward.py:TestIP6Forward::test_success[ll-slow] -> passed [1.685s]
Something can be improved here (avoid IPv6 tentative delays by default) and bring this doesn 300-400ms more or less easily.
I guess the biggest thing here is scapy initialisation (not sure if we actually needs scalpy.all).
[1]: I'm investigating setup and teardown.
Anything you can do to improve python test case runtimes would be appreciated. I run the regression tests in qemu quite frequently (partially because that can be done without any special system privileges) and python tests are very slow there. Parallelization helps somewhat.
Thanks for trying it out, I really appreciate it. pfsync:bulk is indeed mostly test-specific setup /business logic and using python for just one test won't make any benefits. Some cumulative benefit may appear if one writes helpers for the most-often used pf helpers, but thats a matter of the personal preference.
There's a fair criticism of the atf-sh tests that there's a lot of boilerplate, but I find it makes the test more obvious about what's going on. That is, I can read the test as if it were a very simple shell script. Nothing is hidden, and it's clear what setup is being created.
Yep, "what's going on" is a good argument. In the end I guess we all want to be able to easily write (and debug) the tests if/when something goes wrong. I'd also love to hear on what you think should be improved in the python part - I'd really prefer to make it more user-friendly.
| tests/sys/netinet6/test_ip6_forward.py | ||
|---|---|---|
| 15 | Absolutely. Sorry, I didn't have enough time to fully finish it, so I pushed the code to the review as is. | |
| 136 | Originally I thought of IPC between the vnets to serve 2 purposes:
I'll probably try addin some pre-defined commands handlers & leave the ability to have the custom commands. | |
Which class methods get called when is a bit non-obvious. Perhaps it'd be easier to follow if that was driven by annotations rather than function names. So something like
@atf_py.test_case
def test_success(self):
pass
@atf_py.setup_jail("vnet1")
def make_router(self, vnet):
passI also find the way to access interfaces a bit clunky. self.vnet.iface_alias_map["if1"] doesn't quite roll of the tongue, and "if1" confused me initially because I'm used to thinking in terms of epairs. It's very magical, but it should be possible to teach the vnet object to just give me if1 if I do self.vnet.if1 or even self.if1, for arbitrary names (https://stackoverflow.com/questions/31177629/can-i-create-an-object-that-receives-arbitrary-method-invocation-in-python).
I should also be explicitly clear that using Python for vnet tests absolutely has its place for certain types of tests. The carp example is clearly better expressed in Python than it would have been in shell, because I'd either have had to write a separate python script to parse the packet, or resort to tcpdump output parsing with grep.