tests: Require python3 when using Scapy

[jlduran]

Summary

python3 is a symbolic link that points to the current python 3.X
version.

It is possible for a system to have python (python 3.X) without the
python3 (symlink) package.

Test scripts that use Scapy are invoked using python3, so add it as a
required program.

Test Plan

Assuming the current python version is 3.11:

pkg install -y net/scapy
pkg remove -y python3
pkg install -y python310

The tests should skip, there should be no:

stderr:
env: python3: No such file or directory

[kp]

So this is correct, but it's a pretty marginal case that has to be deliberately provoked. I'm not sure it's worth the churn and ongoing maintenance burden to cope with it.

[asomers]

What maintenance burden, @kp? I think it's a worthwhile change. I've run into similar problems myself. I find it easy to have a system where something pulls in, say, lang/python311 but not lang/python3.

[kp]

In D49007#1117529, @asomers wrote:

What maintenance burden, @kp? I think it's a worthwhile change.

I'm probably overstating it a bit, yeah. I mostly want to keep the tests as simple and straightforward as possible, and having to list what should be a dependency of your already listed dependency isn't that.
Still, it's small enough that I'm not going to fight against it. Put me down as a "doesn't actually say no".

I've run into similar problems myself. I find it easy to have a system where something pulls in, say, lang/python311 but not lang/python3.

Why the python ports do this is probably off-topic here, and not something I'm going to be trying to fix.

[jlduran]

I can understand the nuisances of committing to this pattern.
I will eventually catch any missing entries, and fix them, so do not worry at all about them.
In the long, I can propose something like:

require_scapy()
{
    atf_set require.progs python3 scapy
}

In the common subroutine, and use require_scapy in the header, if the verbosity is the burden.

[kp]

In D49007#1118107, @jlduran wrote:

In the long, I can propose something like:

require_scapy()
{
    atf_set require.progs python3 scapy
}

In the common subroutine, and use require_scapy in the header, if the verbosity is the burden.

Interesting idea. Does that extend or replace previous atf_set require.progs lines? (You may not know, I certainly don't. Something we need to test if we go that way.)

[jlduran]

In D49007#1118128, @kp wrote:

In D49007#1118107, @jlduran wrote:

In the long, I can propose something like:

require_scapy()
{
    atf_set require.progs python3 scapy
}

In the common subroutine, and use require_scapy in the header, if the verbosity is the burden.

Interesting idea. Does that extend or replace previous atf_set require.progs lines? (You may not know, I certainly don't. Something we need to test if we go that way.)

So far the plan is to wait for a round of ci.freebsd.org tests to run, then commit this revision, as it has been already approved. Then propose this idea, initially with only netpfil tests.

[ngie]

In D49007#1118128, @kp wrote:

In D49007#1118107, @jlduran wrote:

In the long, I can propose something like:

require_scapy()
{
    atf_set require.progs python3 scapy
}

In the common subroutine, and use require_scapy in the header, if the verbosity is the burden.

Interesting idea. Does that extend or replace previous atf_set require.progs lines? (You may not know, I certainly don't. Something we need to test if we go that way.)

It should replace existing lines where scapy is required. Please use your best judgment about whether or not it should be expressed this way.

[ngie]

In D49007#1117997, @kp wrote:

In D49007#1117529, @asomers wrote:

What maintenance burden, @kp? I think it's a worthwhile change.

I'm probably overstating it a bit, yeah. I mostly want to keep the tests as simple and straightforward as possible, and having to list what should be a dependency of your already listed dependency isn't that.
Still, it's small enough that I'm not going to fight against it. Put me down as a "doesn't actually say no".

I've run into similar problems myself. I find it easy to have a system where something pulls in, say, lang/python311 but not lang/python3.

Why the python ports do this is probably off-topic here, and not something I'm going to be trying to fix.

I don’t think it makes sense to try “fixing” this.

land/python3 is a metaport which requires the default version of python3, per PYTHON_VERSIONS in USES+= python.

[ngie]

I wish the metaport stuff was handled like Debian distros do with the alternatives application :/..

[ngie]

One thing I might do instead of “requires_scapy” is something like “netpfil_required_progs” or set the requirements at the Kyuafile level. Again, this would dramatically reduce the amount of redundant shell code in the ATF testcase headers.

[kp]

In D49007#1118206, @ngie wrote:

It should replace existing lines where scapy is required. Please use your best judgment about whether or not it should be expressed this way.

I meant that to ask about what atf will do.
I.e. given this:

some_head()
{
    atf_set require.progs foo
    require_scapy
}

do we end up requiring python3 and scapy only, or foo as well?

One thing I might do instead of “requires_scapy” is something like “netpfil_required_progs”

It's worth noting that not all netpfil tests (in fact, only a minority) require scapy. We still want to run the tests that can run without scapy if it's not installed.

or set the requirements at the Kyuafile level. Again, this would dramatically reduce the amount of redundant shell code in the ATF testcase headers.

I'm not a fan of setting this in the Makefile (which generates the Kyuafile). It creates distance between the requirement specification and the test code that has the requirement.

[ngie]

In D49007#1118213, @kp wrote:

In D49007#1118206, @ngie wrote:

It should replace existing lines where scapy is required. Please use your best judgment about whether or not it should be expressed this way.

I meant that to ask about what atf will do.
I.e. given this:

some_head()
{
    atf_set require.progs foo
    require_scapy
}

do we end up requiring python3 and scapy only, or foo as well?

IIRC it does the latter.

ATF/kyua would probably benefit from “append” capability with require.progs, etc.

One thing I might do instead of “requires_scapy” is something like “netpfil_required_progs”

It's worth noting that not all netpfil tests (in fact, only a minority) require scapy. We still want to run the tests that can run without scapy if it's not installed.

netpfil_required_progs vs netpfil_scapy_required_progs? These could also be made into readonly shell variables to make it easier to inject/append values..

or set the requirements at the Kyuafile level. Again, this would dramatically reduce the amount of redundant shell code in the ATF testcase headers.

I'm not a fan of setting this in the Makefile (which generates the Kyuafile). It creates distance between the requirement specification and the test code that has the requirement.

I understand the argument, but.. not doing something like this seems like a potentially losing battle over the longterm as folks add, modify, or remove testcases—especially when the person enforcing the policy isn’t always involved in reviews or the decision making process, e.g., out on vacation, no longer with the project, hit by a bus, etc. It gets worse when standards aren’t applied consistently and reviewers provide “ship its” not upholding the higher standards set by another developer.

Beginners will copy/paste lines of code without understanding the why behind them if they’re trying to get a job done quickly. It will only get worse as GenAI produces more templated test code using primitive templates.

Moreover, the larger the change, the longer it takes for reviewers to provide needed feedback on a change.

Although it’s nice having per-test granularity, I think it’s a lot harder to manage, i.e., seeing the forest for the trees.

Sidenote: I’ll have to go back and look as well to see where the requirements enforcement is done, but if it’s done in kyua, it could speed up overall test runtime by reducing common logic/process forking by doing it in kyua instead of ATF.

[ngie]

FWIW, I think the real bug might be that net/scapy requires python3 instead of being properly flavored using PY_FLAVORS, or the shebangs not being updated in the port to use the python3.10 instead of python3, etc.

[jlduran]

I have not forgotten about this revision, I had to see a few tests fail first on ci, and ci has been out for maintenance.
I'll be committing it shortly.
Thank you for this interesting discussion, I will not pursue the require_scapy method because there is no way to append progs to require.progs.