On Fri, Jul 07, 2023 at 07:42:39PM +0200, Stefano Brivio wrote:On Wed, 5 Jul 2023 13:27:17 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:Ok.On Wed, Jul 05, 2023 at 02:30:48AM +0200, Stefano Brivio wrote:I don't think it's really needed, I just wanted to make sure we have a common understanding.Sorry for the delay, some (partial) feedback and a few ideas: On Tue, 27 Jun 2023 12:54:28 +1000 David Gibson <david(a)gibson.dropbear.id.au> wrote:Yeah, I guess not. My point here is that given that we're generally trying to avoid NAT, it seems to me we should primarily test the no-NAT case, then have specific tests for the NAT cases. I'll try to find a better way to phrase that.Convert the old-style tests for pasta (DHCP, NDP, TCP and UDP transfers) to using avocado. There are a few differences in what we test, but this should generally improve coverage: * We run in a constructed network environment, so we no longer depend on the real host's networking configuration * We do independent setup for each individual test * We add explicit tests for --config-net, which we use to accelerate that setup for the TCP and UDP tests * The TCP and UDP tests now test transfers between the guest and a (simulated) remote site that's on a different network from the simulated pasta host. This better matches the typical passt/pasta usecase...this is not necessarily true -- it really depends, but sure, it's important to have this too.Right... there's two aspects to that too. In bats we have directives giving the structure of the testsuite - that is listing which test is which, then the contents of each test is more or less just code. It would also be possible to have helper directives within the test, although in that case it doesn't really differ than much from just being a function call in whatever language it is. Is it one of those cases specifically you're looking at? It would be not that complex to redesign the "avocado-classless" plugin thingy to be more declarative in terms of defining tests, which might make it more appealing to you. I think the major trade-off of doing so is that that would force the tests to occupy a different file from e.g. test library code. That's no real change for the "real" tests of passt & pasta, but would make things a bit more clunky for the tests of the test library.True, even though there's also the approach, quite commonly implemented in test suites, of mixing actual code with custom directives or annotations (e.g. bats does that) to keep things a bit simpler or more descriptive. But I can't come up with a reasonably good way here, and it's probably doable to also add this later on, maybe in form of pre-processing.Right, I see/share your concern. To some extent there's an unavoidable trade-off here. Avoiding lots of repetition in the test cases, leads to "code" in some sense: we need something resembling function calls and loops at least to re-invoke standard test pieces.Signed-off-by: David Gibson <david(a)gibson.dropbear.id.au> --- oldtest/run | 14 ++-- test/Makefile | 1 + test/avocado/pasta.py | 129 ++++++++++++++++++++++++++++++++++ test/lib/layout | 31 -------- test/lib/setup | 40 ----------- test/pasta/dhcp | 46 ------------ test/pasta/ndp | 33 --------- test/pasta/tcp | 96 ------------------------- test/pasta/udp | 59 ---------------- test/run | 8 --- test/tasst/dhcpv6.py | 4 +- test/tasst/pasta.py | 42 +++++++++++ test/tasst/scenario/simple.py | 44 ++++++------ 13 files changed, 203 insertions(+), 344 deletions(-) create mode 100644 test/avocado/pasta.py delete mode 100644 test/pasta/dhcp delete mode 100644 test/pasta/ndp delete mode 100644 test/pasta/tcp delete mode 100644 test/pasta/udp create mode 100644 test/tasst/pasta.py diff --git a/oldtest/run b/oldtest/run index a16bc49b..f1157f90 100755 --- a/oldtest/run +++ b/oldtest/run @@ -70,13 +70,13 @@ run() { test build/clang_tidy teardown build -# setup pasta -# test pasta/ndp -# test pasta/dhcp -# test pasta/tcp -# test pasta/udp -# test passt/shutdown -# teardown pasta + setup pasta + test pasta/ndp + test pasta/dhcp + test pasta/tcp + test pasta/udp + test passt/shutdown + teardown pasta # setup pasta_options # test pasta_options/log_to_file diff --git a/test/Makefile b/test/Makefile index 953eacf2..9c3114c2 100644 --- a/test/Makefile +++ b/test/Makefile @@ -227,6 +227,7 @@ $(VENV): .PHONY: avocado-assets avocado-assets: nstool small.bin medium.bin big.bin + $(MAKE) -C .. pasta .PHONY: avocado avocado: avocado-assets $(VENV) diff --git a/test/avocado/pasta.py b/test/avocado/pasta.py new file mode 100644 index 00000000..891313f5 --- /dev/null +++ b/test/avocado/pasta.pyI'm just focusing on this for the moment, as this is the part I'm mostly concerned about (writing tests), with a particular accent on what makes this (still) read-only _code_ for me, and the gaps with the kind of things I would have naturally expected to write. That is, I'm mostly isolating negative aspects here. I'm stressing "code" because I also would have the natural expectation that it should/must be simpler to _write_ (not necessarily design) tests rather than the code that end users use, because we can use whatever language with no strict (only some) constraints on resources and speed. So in my opinion it doesn't necessarily need to be "code" (and a general feeling I have from this is that it really is code).Ok, I've started moving towards that for the next draft.That's not to say we can't make it better than it is in this draft.Ah, that's nicer. One would still copy and paste it, but without thinking too much.Fair point. I was considering making the idiom here: from avocado_classless.test import * Which will import "all" (in fact a curated list) of things from the module for use here. I didn't go that way because pylint whinged about it, but we could suppress that easily enough.@@ -0,0 +1,129 @@ +#! /usr/bin/env avocado-runner-avocado-classless + +# SPDX-License-Identifier: GPL-2.0-or-later +# +# Copyright Red Hat +# Author: David Gibson <david(a)gibson.dropbear.id.au> + +""" +avocado/pasta.py - Basic tests for pasta mode +""" + +import contextlib +import ipaddress +import os +import tempfileSo far so good, I probably don't need to read up about every single import.+from avocado_classless.test import assert_eq, assert_eq_unordered, testProbably a future source of (needless) copy-and-paste.I'll see what I can do along these lines. One thing that might get messy is the question of which things want a bare address (127.0.0.1), and which things what an address+netmask (127.0.0.1/8). Although, TBH, that's already kinda messy.I'm just saying that either we "magically" import things by pre-processing test scripts or suchlike, or we have an explicit import, but no matter what, we need that. The current framework 1. doesn't implement anything like that and 2. it wouldn't need an "import" anyway because it's a domain-specific language. But here, we need that, and probably more, and I guess it's reasonable.Of course, assert_eq_unordered is not available in the current tests, and we need it, plus a syntax to represent that.I'm not quite sure what you're getting at here.To me treating it as string doesn't sound like a bad idea, except when we want to do calculations/masks (I guess that would be the most problematic part?)... but then we could probably convert to "addresses" on the fly as needed?Still, conceptually speaking, we shouldn't need this kind of import if we are writing _a test for passt in the test suite for passt itself_, and we want to check that the list of interfaces matches "lo" -- it's something we'll need basically everywhere anyway.Right... so "LOOPBACK6" is longer than "::1", but not than "ipaddress.ip_address('::1')". That is, these helpers produce something that's in an actual IP address type, rather than a bare string. I could try moving to just using strings for IP addresses throughout the tests, but that might cause some different messes.+from tasst.address import LOOPBACK4, LOOPBACK6On one hand, probably more readable than a 127.0.0.1 here and there (even though longer than "::1"), on the other hand there are 256 loopback addresses in IPv4. And there is only one way of writing "::1", but many ways of remembering/misspelling LOOPBACK6.Hrm.. where, though. The point is that this is a technique I'm using heavily throughout, not just in a particular place.Right -- maybe a comment could help, then.Right, I could introduce "from foo import *" conventions for some of these too.+from tasst.dhcp import test_dhcp +from tasst.dhcpv6 import test_dhcpv6 +from tasst.ndp import test_ndp +from tasst.nstool import unshare_site +from tasst.pasta import Pasta +from tasst.scenario.simple import ( + simple_net, IFNAME, HOST_NET6, IP4, IP6, GW_IP4, REMOTE_IP4, REMOTE_IP6 +) +from tasst.transfer import test_transfersSimilar for this -- it would be great to have an infrastructure where we can just use what we need without looking it up. Rather minor though.Yeah. I agree that this is uncomfortably cryptic, however this one I don't see an easy way to remove. I heavily use this to conveniently modify and extend context managers. It allows two things which are both very convenient: 1) You can do: @contextlib.contextmanager def foo(...): setup() yield whatever # <== actual test runs here cleanup() this will do the cleanup() even if the test blows up. 2) You can do @contextlib.contextmanager def bar(...): with other_context_1(): with other_context_2(): yield whatever # <=== actual test runs here This effectively makes this a combination of the two existing contexts, again ensuring that both their cleanup happens if the test blows up. It's certainly possible to do this with direct implementations of the contextmanager protocol but it's much more verbose, and not significantly less esoteric.+ + +IN_FWD_PORT = 10002 +SPLICE_FWD_PORT = 10003 +FWD_OPTS = f'-t {IN_FWD_PORT} -u {IN_FWD_PORT} \ + -T {SPLICE_FWD_PORT} -U {SPLICE_FWD_PORT}'I think clear enough.+ +(a)contextlib.contextmanagerAbout this decorator: I now have a vague idea what it means after reading the full series and some Avocado documentation... but I don't find that very descriptive.Right. This (so far) is the case where we explicitly create a guest ns for pasta, rather than having pasta make one for us. We want that to be a child of the ns which is simulating the "host" for pasta, and that "simhost" ns is created as part of simple_net().Oh, I didn't realise that unshare_site() needs stuff from simple_net(). -- then this makes sense.Hmm.. I'm not sure what's throwing you on this specifically. We need both pieces of setup for our test, so we need to nest them within each other. They have to go in this order because the parameters to unshare_site() need something from the simnet.+def pasta_unconfigured(opts=FWD_OPTS):Clear.+ with simple_net() as simnet:Also clear.+ with unshare_site('pastans', '-Ucnpf --mount-proc', + parent=simnet.simhost, sudo=True) as guestns:If I ignore for a moment the implementation detail, it's not very intuitive why this is nested in the "with" statement before.And yes, this itself would be clearer in my opinion if we then had "tempfile.TemporaryDirectory()" on the same level as you mention later, because otherwise I'm reading it as something nested, because the language wants us to do that. It's not the case, these two are conceptually nested.So, in this case I nested it because I hadn't hit the bits that needed the tmpdir until this point. I probably could get both the simple_net and the tmpdir as part of the same outermost 'with'. I'll take a look.I still think it would be easier to read this nesting in a YAML file or suchlike. But of course this part of a different (and larger) problem, so maybe we can just live with this for the moment...Ok. I'll move towards using keyword arguments everywhere.Yes, definitely, a bit more to type, but absolutely understandable.Those parameters require a complete understanding of unshare_site(),Hm, ok. I could require that this takes keyword arguments, so it would be something like: unshare_site(name='pastans', unshare_opts='-Ucnpf --mount-proc', ..) Would that help?Ok, I'll have a look at this.'keep_caps' does what it says, that would definitely help.and... I now know what "sudo=True" means in Avocado, but I don't even have sudo on my machine, so at a distracted look I would think I need to install it and configure sudoers (which would make me as a developer skip the tests).Yeah. And it's arguably not entirely accurate either. I could rework the 'site' stuff so that I call this, say, 'capable' or 'keep_caps' instead. Would that help?You mean take some sort of Python description of what we want to unshare, then generate the unshare(1) options from that? I guess we could, I'm just a bit dubious about the worth of doing that when we don't actually need to do anything other than pass some options verbatim to unshare(1).Yes, okay, fair point.So far, what this says is: - use addressing from simple_net()Hrm, it's more than that: simple_net() is actually *creating* the simulated simple network (and cleaning it up once the with clause ends).Right, sorry, I meant capabilities. While at it, would it perhaps make sense to build unshare(1) options in unshare_site() itself?- detach user and network namespace, remounting proc in it, preserve (I guess?) credentialsWell, if by "preserve credentials" you mean "keep the capabilities you have because you own the namespace", yes (this is equivalent to "nsenter --keep-caps", not "nsenter --preserve-credentials").Right, and it's at the granularity of a whole script/process. 'with' gives us full control of the scope in which we need the setup.Okay, fair, POSIX shell doesn't have that (it has traps, but you need to pop/push stuff from them, which means it's less terse anyway).So it's not quite equivalent to tmpdir=$(mktemp -d): it does that at the start of the with, but it also removes the directory at the end of the with.+ with tempfile.TemporaryDirectory() as tmpdir:...even less intuitive why this statement (tmpdir="$(mktemp -d)") is nested.Right. Basically I feel like something processing this would more or less be translating a heirarchy of thingies in yaml to a heirarchy of function calls to do the setup. That seems like a fair bit of boilerplate hassle to achieve a pretty small improvement in syntax readability over just making those function calls directly.We need the network constructed, the namespace created and the temporary directory available all at the same time. So the scope of each 'with' context needs to overlap - in other words to be nested. In this case the order of nesting doesn't matter, we could put the temporary directory 'with' on the outside.This last part is quite hard for me to grasp. But maybe an over-commented test "template" would fix it.Yes, specifically, start pasta with it's "host" being simnet.simhost, and its "guest" being guestns. Again I could enforce keyword parameters to make the meaning of the arguments a bit clearer. [Aside, I plan to extent Pasta() so that if you don't pass a guest ns it will default to having pasta create its own - that ns will still be available as 'pasta.ns', so the rest of the stuff can remain the same] Then the "yield" essentially means "now that we've done the setup, here is where to run the actual specific test".+ pidfile = os.path.join(tmpdir, 'pasta.pid') + + with Pasta(simnet.simhost, pidfile, opts, ns=guestns) as pasta: + yield simnet, pasta.ns...and this finally says: start pasta in that (user? network?) namespace.setup: net: not_so_simple_net site_a: pasta_options: -t 10002 -T 10003 -u 10002 -U 10003 start pasta site_b: pasta_options: -t 10012 -T 10013 -u 10012 -U 10013 start pasta ?So, from my expectation, pseudocode: setup: net: simple_net pasta_options: -t 10002 -T 10003 -u 10002 -U 10003 start pastaHrm, that pseudocode assumes a pretty specific set of possible setups, and how they relate to each other. For example I can't see how it would extend to running two different pasta instances on different simulated hosts.That's possible with the approach I'm using here.Sure, and it's less effort compared to what I'm describing, I know.I'm not sure what you mean by that.Still, I think the equivalent in POSIX shell (we kind of have it already) would be clearer. Obvious downside: a lot more to write, and it's bug-prone. Maybe we could solve this particular issue by a very verbosely documented example.and from my understanding of what this is replacing: setup_pasta() { context_setup_host unshare context_run_bg unshare "unshare -rUnpf ${NSTOOL} hold ${STATESETUP}/ns.hold" context_setup_nstool ns ${STATESETUP}/ns.hold # Ports: # # ns | host # ------------------|--------------------- # 10002 as server | spliced to ns # 10003 spliced to init | as server context_run_bg passt "./pasta -f -t 10002 -T 10003 -u 10002 -U 10003 -P ${STATESETUP}/passt.pid $(${NSTOOL} info -pw ${STATESETUP}/ns.hold)" }It's replacing not just that, but also the matching teardown function, and ensures that the teardown runs if there's an exception in the test itself (including running the teardowns for the outer contexts if the setup of the inner contexts failed).I guess needed.which I actually find less elegant but much clearer, especially before reading the whole series, I wonder if we could have either a configuration format, or directives... but, disappointingly, I couldn't decide on a more detailed proposal yet. Still, I would aim at something resembling that pseudocode/configuration above. Of course, with no details it looks neat, so the comparison is unfair, but I'm under the impression that even after adding details (that's what I'm trying to figure out/articulate) it should still be much clearer.I'm just not convinced that it would be, without losing a great deal of the flexibility in this system. Or at least, not clearer than a more polished version of this proposal.We could put a description in a docstring easily enough.+ + +@test +def test_ifname():I guess fine (even though it's missing a descriptive name _inline_).Hm, can't really do that - "yield" is a keyword, not a function or identifier.I'm not entirely sure what that entails, but I guess it might be more readable overall (also because we'll probably need more fields...? How do you explicitly get a reference to a PID, for example?)Yeah. One of the bits I'm least happy with is what to do in cases where the information the setup needs to pass to the tests proper is quite complex: so here it has both the simulated network which is hosting the pasta instance, and also the namespace within pasta. This specific test doesn't need the former, hence the _. We could use a suitably named dummy variable which might be a bit more readable, though it would need a lint suppression (unused variable). I've considered another approach, though I'm not sure it's an improvement, which is to use more intermediate structs/classes. So, in that approach pasta_unconfigured() would give you a 'PastaTestScenario' instance with fields for the guest ns, host ns and whatever else seems useful.+ with pasta_unconfigured() as (_, ns):Ouch... a bit hard to grasp for people not familiar with Python or Go... plus, conceptually, we _just_ want to say either:Argh. Yes.test: "pasta namespace has the interface name we configured" enter namespace list of interfaces includes $NAME or: test: "pasta namespace has a loopback interface" list of interfaces in namespace includes $NAMEIt's more than that though: we're also requesting a very specific (simulated) environment for the test, which the pseudocode above just kind of assumes. 'pasta_unconfigured()' (name certainly negotiable) is handling the setup and teardown of everything we need for a certain class of pasta tests. That has 3 namespaces, a veth, and a pasta instance, all configured in a particular way....and yes, that requires that we start pasta first, but I wonder if it makes sense to mix the two notions (i.e. whether an object-oriented approach makes sense at all, actually).So the OO approach works ok for pretty simple setups - probably why it became popular with jUnit etc: the class handles the setup, the methods do the specific tests. But it breaks down pretty quickly: - If you want to use multiple "setup" pieces at once you need to use multiple inheritence, which is a nightmare- If you want to parameterize parts of the setup, you need to do it at the whole class level which involves wierd non-local interactions - If you want to use multiple setup pieces which are actually the same piece repeated with different parameters, you're pretty much out of luck I realize that context managers aren't the most obvious thing, but they have the huge advantage that every test can directly and reasonably tersely declare what setup they need to run: def test(): with setup_i_need(): with other_setup_i_need(): with setup_i_need(param='non default'): # actually do the test This also handles the necessary teardown at each level, and it still works if some of the inner setups need paramaters that are derived fromt the outer setups.Yes yes definitely.Probably easier to understand, but it doesn't solve the problem that "yield" doesn't mean "we're checking this". Maybe it could be worked around with naming though, I haven't tried.It's not the same as list includes (that's assert_in()). This checks that the lists (strictly, iterables) are identical ignoring order. So in this case ['lo', IFNAME] and [IFNAME, 'lo'] would be ok, but ['lo', 'eth99', IFNAME] would not be.+ assert_eq_unordered(ns.ifs(), ('lo', IFNAME))Clear (probably clearer with a "list includes" operator).Yeah, I'm not super happy with this one. In particular I dislike the inconsistency: in simple tests like the above we have, loosely the setup, followed by the test. In these composed examples we have the tests then the setup. So combining a few options I've thought of, I could do something like this: @contextlib.contextmanager def pasta_ndp(): with pasta_unconfigured() as (simnet, guestns): guestns.ifup(IFNAME) yield NdpTestScenario(ifname=IFNAME, net=HOST_NET6, router=simnet.gw_ip6_ll.ip, ndpclient=guestns) compose_matrix([pasta_ndp], NDP_TESTS) Here NDP_TESTS would come from the ndp module and have the list of individual ndp test cases, each of which takes an NdpTestScenario as parameter.+ + +@test_ndp(IFNAME, HOST_NET6) +(a)contextlib.contextmanager +def pasta_ndp(): + with pasta_unconfigured() as (simnet, guestns): + guestns.ifup(IFNAME) + yield guestns, simnet.gw_ip6_ll.ipThis really hides what we are checking. Other than that (but it's a big issue in my opinion) I find it terse and elegant.Ok, I'll move towards that throughout.Much better, I think.Right. So would the treatment above help? Enough? It would look something like: @contextlib.contextmanager def pasta_dhcp(): .... yield DhcpTestScenario(ifname=IFNAME, client_ip4=IP4.ip, server_ip4=GW_IP4.ip, mtu=65520) compose_matrix([pasta_dhcp], DHCP_TESTS)+ +@test_dhcp(IFNAME, IP4.ip, GW_IP4.ip, 65520)The test_ndp decorator is probably usable, this one not so much. As we have more parameters, it would probably be better to have something more descriptive (even if necessarily more verbose).Well.. sort of. The idea here is that 'TransferTestScenario' doesn't itself *do* anything - it's just a container for all the information relevant to running transfer tests in a specific setup. All the individual transfer test functions then use the information in here to do their thing.Ah, right, that rings a bell.It is needed, and yes we probably should, I haven't had a chance to look into what, exactly. We may hit that kernel bug I noticed where permissions to the disable_dad sysctl seem to behave weirdly.+@test_dhcpv6(IFNAME, IP6.ip) +(a)contextlib.contextmanager +def pasta_dhcp(): + with pasta_unconfigured() as (_, guestns): + yield guestns + + +(a)contextlib.contextmanager +def pasta_configured(): + with pasta_unconfigured(FWD_OPTS + ' --config-net') as (simnet, ns): + # Wait for DAD to complete on the --config-net addressSide note: this shouldn't be needed -- if it is, we should probably fix something in pasta.Yes, now I actually understand what outward_transfer() does: it calls TransferTestScenario(). I really had no idea otherwise.I'm not entirely sure what you mean here. Would the same treatment as for the ndp and dhcp cases above help? @contextlib.contextmanager def outward_transfer(): with pasta_configured() as (simnet, ns): yield TransferTestScenario(client=ns, server=simnet.gw, connect_ip4=REMOTE_IP4.ip, connect_ip6=REMOTE_IP6.ip, connect_port=10000) # variants for inward transfer, and spliced_transfer compose_matrix([outward_transfer, inward_transfer, spliced_transfer], TRANSFER_TESTS)+ ns.addr_wait(IFNAME, family='inet6', scope='global') + yield simnet, nsExcept for the common points with my observation above, this looks usable, and:+ + +@test +def test_config_net_addr(): + with pasta_configured() as (_, ns): + addrs = ns.addrs(IFNAME, scope='global') + assert_eq_unordered(addrs, [IP4, IP6]) + + +@test +def test_config_net_route4(): + with pasta_configured() as (_, ns): + (defroute,) = ns.routes4(dst='default') + gateway = ipaddress.ip_address(defroute['gateway']) + assert_eq(gateway, GW_IP4.ip) + + +@test +def test_config_net_route6(): + with pasta_configured() as (simnet, ns): + (defroute,) = ns.routes6(dst='default') + gateway = ipaddress.ip_address(defroute['gateway']) + assert_eq(gateway, simnet.gw_ip6_ll.ip) + + +@test +def test_config_net_mtu(): + with pasta_configured() as (_, ns): + mtu = ns.mtu(IFNAME) + assert_eq(mtu, 65520)these all make intuitive sense to me.+ + +(a)test_transfers(ip4=REMOTE_IP4.ip, ip6=REMOTE_IP6.ip, port=10000) +(a)contextlib.contextmanager +def outward_transfer(): + with pasta_configured() as (simnet, ns): + yield ns, simnet.gwThis, however, not. I would naturally expect the function implementing a template of data transfer test to do something with data.Indeed, and that's not where the decorators are arising from. With the revisions I'm already planning on there will only be two decorators: @test(), which is used to declare a particular function as a standalone test (possibly with options like timeout). This could be done instead with explicit "register_test('name', testfn)" calls, but the decorator form seems more natural to me. @contextlib.contextmanager, which is a way of much more tersely writing "context managers", which are essentially a matched pair of setup/teardown functions. I realize that they're kind of obscure, but it's a huge reduction in the amount of boilerplate: Suppose we have an existing setup foo(), and want to extend it with one extra step. Using the decorator it's: @contextlib.contextmanager def bar(param): with foo(param) as f: additional_setup(f) yield f To open code it would be roughly: class Bar(contextlib.AbstractContextManager): def __init__(self, param): self.foo = foo(param) def __enter__(self): f = self.foo.__enter__() try: additional_setup(f) except Exception as e: self.foo.__exit__(... not sure what goes here ...) return f def __exit__(self, *exc_info): self.foo.__exit__(*exc_info) .. and it just gets worse with more complex examples.Hmm, yes, I guess that would be better than the alternative I was afraid of (extending pasta_configured()).So, I was only thinking of pasta_configured() and pasta_unconfigured() as useful for the pretty specific pattern of pasta invocations in this batch of tests (which is why they're local to that file). I've also had at pasta_options (which I'm thinking of as the log-to-file tests, since that's all they test so far). I was planning a new helper to set up a suitable scenario for that, which would be based on simple_net() and Pasta() with some kind of parameterization for where to place the logfile.+ + +(a)test_transfers(ip4=IP4.ip, ip6=IP6.ip, port=IN_FWD_PORT, + fromip4=REMOTE_IP4.ip, fromip6=REMOTE_IP6.ip) +(a)contextlib.contextmanager +def inward_transfer(): + with pasta_configured() as (simnet, ns): + yield simnet.gw, ns + + +@test_transfers(ip4=LOOPBACK4, ip6=LOOPBACK6, port=SPLICE_FWD_PORT, + listenip4=LOOPBACK4, listenip6=LOOPBACK6) +(a)contextlib.contextmanager +def spliced_transfer(): + with pasta_configured() as (simnet, ns): + yield ns, simnet.simhostI still have to decode these before I can reasonably comment on them. I'll follow up with a more detailed proposal of a possible configuration format or perhaps something between a domain-specific language and an annotated Python script (à la bats), but I'm trying to consider a few more tests on top of these. I started looking at "options" tests next, and realised my proposal was a bit too simplistic. But also that the current version of those tests, while somewhat verbose and surely clunky, shows in a very clear way what we're checking and what we expect... and I'm not sure that the outcome from extending pasta_configured() would be very usable.Eh, yes, but if you take this part by itself, you wouldn't naturally jump to Python decorators. :)All we need for that is: - define options and network model/addressingI think that short statement hides a lot of complexity.-- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson> - have a clear syntax of identifying where pasta is starting > > - a list of commands (which, themselves, are absolutely obvious and > natural in a shell script, but I see the value of using Python > features to check assertions, at least).