In D38053#876436, @jlduran_gmail.com wrote:

Yes, please!
In the meantime, I'll keep updating this "BASE" revision.
Changes:

• Add the ability doctor IHL fields
• Add the ability to remove inner packet load

I'm sorry for the delay, was focusing on fixing staff for the upcoming 13.2 release.

Yes, please!
In the meantime, I'll keep updating this "BASE" revision.
Changes:

• Add the ability doctor IHL fields
• Add the ability to remove inner packet load

This revision is up for a while. What do you folks think of committing some or all of these tests that don't fail or current ping?
Nothing stops us from committing the other tests (or fixing some of the current tests) along with the ping changes or separately.

• Add a missing extra white space

The output was originally taken from a branch that has code that trims these extra spaces. Let's leave those cosmetic changes for last.

• Do not shadow opts var
• Disable net.inet.ip.process_options for RR options as well (this was discovered by randomizing the tests, as this sysctl was set by the previous tests)

Updates:

• Use ToolsHelper.set_sysctl
• Use type hints
• Skip failing tests
• Remove tests of features not yet implemented

In D38053#872961, @ngie wrote:

In D38053#867963, @jlduran_gmail.com wrote:

In D38053#867921, @melifaro wrote:

3. An atf_get_srcdir-equivalent to eventually read files relative to the source directory, however I think the idea is to disassociate from ATF later on.

This one slipped through the cracks, thanks for reminding! Will add in a day or two.

I thought of passing it via wrapper initially, but ended up with a bit more natural way - getting it from pytest itself.
I committed an example here.

Ha, very nice! I tend to prefer the f-string format (i.e., print(f" {k}: {v}"), is there a preference in style?

f-strings are fine given the FreeBSD v cpython support matrix, as long as you use 3.8+ only extensions to f-strings.

@ngie Thank you for your suggestions, I'll try to implement them!
I also plan to update this file, hopefully this week, I'll remove/skip the tests for D37930 (as it hasn't landed yet), and start submitting the proposed fixes for the utility itself. I don't think all of them will be able to make it to the upcoming release, so there isn't really a time constraint, but I would like to start receiving feedback on that as well. Admittedly, having nice tests usually facilitates it!

In D38053#867963, @jlduran_gmail.com wrote:

In D38053#867921, @melifaro wrote:

3. An atf_get_srcdir-equivalent to eventually read files relative to the source directory, however I think the idea is to disassociate from ATF later on.

This one slipped through the cracks, thanks for reminding! Will add in a day or two.

I thought of passing it via wrapper initially, but ended up with a bit more natural way - getting it from pytest itself.
I committed an example here.

Ha, very nice! I tend to prefer the f-string format (i.e., print(f" {k}: {v}"), is there a preference in style?

In D38053#867921, @melifaro wrote:

3. An atf_get_srcdir-equivalent to eventually read files relative to the source directory, however I think the idea is to disassociate from ATF later on.

This one slipped through the cracks, thanks for reminding! Will add in a day or two.

I thought of passing it via wrapper initially, but ended up with a bit more natural way - getting it from pytest itself.
I committed an example here.

In D38053#867961, @melifaro wrote:

4. In the meantime the kernel delayed object reclamation model issue is fixed, maybe adapt 80fc25025ffcb0d369fc0b6d4d272ad6fd3f53c3 ?

Committed in 20ea7f26e413 .

In D38053#867921, @melifaro wrote:

3. An atf_get_srcdir-equivalent to eventually read files relative to the source directory, however I think the idea is to disassociate from ATF later on.

This one slipped through the cracks, thanks for reminding! Will add in a day or two.

I thought of passing it via wrapper initially, but ended up with a bit more natural way - getting it from pytest itself.
I committed an example here.

Ha, very nice! I tend to prefer the f-string format (i.e., print(f" {k}: {v}"), is there a preference in style?

4. In the meantime the kernel delayed object reclamation model issue is fixed, maybe adapt 80fc25025ffcb0d369fc0b6d4d272ad6fd3f53c3 ?

Committed in 20ea7f26e413 .

3. An atf_get_srcdir-equivalent to eventually read files relative to the source directory, however I think the idea is to disassociate from ATF later on.

This one slipped through the cracks, thanks for reminding! Will add in a day or two.

I thought of passing it via wrapper initially, but ended up with a bit more natural way - getting it from pytest itself.
I committed an example here.

In D38053#867487, @ngie wrote:

In D38053#864580, @asomers wrote:

This looks like a good start. I'm not a fan of the methodize stuff, though. I think it makes the tests harder to read and harder to search. Is there a way to parameterize the tests without using methodize, even if it becomes slightly more verbose?

If you’re referring to “pytest.marks.parametrize”, it supports an optional id= parameter which better describes tests for humans.

In D38053#864580, @asomers wrote:

This looks like a good start. I'm not a fan of the methodize stuff, though. I think it makes the tests harder to read and harder to search. Is there a way to parameterize the tests without using methodize, even if it becomes slightly more verbose?

In D38053#866669, @melifaro wrote:

In D38053#865570, @jlduran_gmail.com wrote:

Thank you! I'll wait for @asomers comments to update the revision.

I'll document here my small - not really important - wishlist:

1. Optional randomization of the test order, maybe something similar to pytest-random-order.

Nice one, can probably work out-of-the box, but the scope will be a single file, due to the way of ATF<>pytest interaction is implemented.

2. When debugging atf-python tests, the output could be a little bit less busy, maybe pytest_terminal_summary et al. instead of print() could be one answer.

I get the intent and I agree. Maybe you could come up with an example (or even a diff) on how do you see it?

Sure! Once all ping tests are added, I should be more acquainted with the testing framework and propose a revision.

3. An atf_get_srcdir-equivalent to eventually read files relative to the source directory, however I think the idea is to disassociate from ATF later on.

This one slipped through the cracks, thanks for reminding! Will add in a day or two.

4. In the meantime the kernel delayed object reclamation model issue is fixed, maybe adapt 80fc25025ffcb0d369fc0b6d4d272ad6fd3f53c3 ?

Yep, been on my list for quite some time, I'll do the change in a day or two.

Great!

In D38053#865570, @jlduran_gmail.com wrote:

Thank you! I'll wait for @asomers comments to update the revision.

I'll document here my small - not really important - wishlist:

1. Optional randomization of the test order, maybe something similar to pytest-random-order.

Nice one, can probably work out-of-the box, but the scope will be a single file, due to the way of ATF<>pytest interaction is implemented.

2. When debugging atf-python tests, the output could be a little bit less busy, maybe pytest_terminal_summary et al. instead of print() could be one answer.

I get the intent and I agree. Maybe you could come up with an example (or even a diff) on how do you see it?

3. An atf_get_srcdir-equivalent to eventually read files relative to the source directory, however I think the idea is to disassociate from ATF later on.

This one slipped through the cracks, thanks for reminding! Will add in a day or two.

4. In the meantime the kernel delayed object reclamation model issue is fixed, maybe adapt 80fc25025ffcb0d369fc0b6d4d272ad6fd3f53c3 ?

Yep, been on my list for quite some time, I'll do the change in a day or two.

Thank you! I'll wait for @asomers comments to update the revision.

So generally, it looks good to me, and I'm fine with committing the change. @asomers: what do you think?

In D38053#865288, @melifaro wrote:

In D38053#865132, @jlduran_gmail.com wrote:

Here are the last iteration changes:

• Use a dictionary for expectations — this keeps both ping and pinger tests inline. Comparing the expected with the actual subprocess.CompletedProcess was also not feasible. It also was suggested initially by @melifaro.

One thing I noticed is that atf-python tests are slower than atf-sh, this was somewhat expected, but when all tests run, the total time builds up. I believe the gains are really from the development perspective.

Q: what's the runtime? e.g. median time reported by kyua test?

In D38053#865132, @jlduran_gmail.com wrote:

Here are the last iteration changes:

• Use a dictionary for expectations — this keeps both ping and pinger tests inline. Comparing the expected with the actual subprocess.CompletedProcess was also not feasible. It also was suggested initially by @melifaro.

One thing I noticed is that atf-python tests are slower than atf-sh, this was somewhat expected, but when all tests run, the total time builds up. I believe the gains are really from the development perspective.

Q: what's the runtime? e.g. median time reported by kyua test?
There are some tests in net/routing, doing similar SingleVnet-isolated testing, written both in python and in C.
C version takes ~120ms per test, python version takes ~330ms. My take is that the runtime under 0.5 second is fine. It may also be not easy to decrease those 330ms, as pytest does a lot of preparation work, but the runner calls it for a single test.
I suspect that importing scapy.all may contribute to the delay here. Note that kyua needs to first run list for the tests (to determine the isolation details) and then actual test and cleanup procedures.
Importing scapy.all in the top of the file will cause all 3 cases to wait till scapy init. I'd probably try to check if it's possible to use scapy subset and/or try to load it only when needed.

Here are the last iteration changes:

Updates:

• Simple one-line ping parameter expectations are subprocess.CompletedProcesses. No need to have a custom class (ExpectedProcess) that does essentially the same thing.
• Incorporate the code from D37876. The pinger utility (now a function) returns a subprocess.CompletedProcess as well. Parametrization and cleanup are next.

In D38053#864674, @melifaro wrote:

Thank you for addressing the comments!
I’d still prefer to have the ids embedded explicitly via pytest.param, but I don’t insist on doing it.

Generally there may be somewhat conflicting ideas on what’s the “best” approach here, but personally I don’t think it’s a big deal here. This is a test code, it already is reasonably simple and allow for easy extension or debugging - that’s “good enough” and that’s what matters most (to me).

Address more suggestions:

• Embed the ids explicitly via pytest.param().

Thank you for addressing the comments!
I’d still prefer to have the ids embedded explicitly via pytest.param, but I don’t insist on doing it.

Address some of the suggestions:

In D38053#864580, @asomers wrote:

This looks like a good start. I'm not a fan of the methodize stuff, though. I think it makes the tests harder to read and harder to search. Is there a way to parameterize the tests without using methodize, even if it becomes slightly more verbose?

Conceptually LGTM, please see some comments on the structure

This looks like a good start. I'm not a fan of the methodize stuff, though. I think it makes the tests harder to read and harder to search. Is there a way to parameterize the tests without using methodize, even if it becomes slightly more verbose?

Update to 8.0.0.18

• Update WWW: to Makefile

• Change versioning to handbook: Example 19. Using USE_GITHUB to Access a Commit Between Two Versions
• Update WWW: to Makefile

@dereks_lifeofadishwasher.com Can we the/an existing version format that upstream uses (vX.Y.Z), optionally suffixed appropriately with relevent git tag if its 'in between versions' ?

Move to a GH_TAGNAME from the released 8.0.0 to include some IMAP fixes
All tests pass.

• Use USES=pytest

portfmt sort order

Yes, that works well. Thanks for the recommendation.

• update for USES=pytest