testing: add python test examples.

[melifaro]

Summary

Simplify the adoption of python tests by proving some example tests, utilising commonly-used patterns.

[jlduran]

Thank you!
Certainly, much better than a wiki.
Is this the desired output:
https://cirrus-ci.com/task/6234303340740608 ?

tests/examples/Makefile

5

I believe this directory is not created when make all install standalone? but that is tangential to this revision. I suspect ATF_TESTS_PYTEST.

tests/examples/test_examples.py

41

98

There has been a preference lately for RFC 5737 addresses, to avoid a possible clash with a private IP currently in use.

114

177

185

[jlduran]

Couple more nits.
New run (this time as expected):
https://cirrus-ci.com/task/5756291799842816?logs=test_examples
I'll eventually add a simple example that consumes Scapy directly (instead of doing it via a shell script).

tests/examples/test_examples.py

53

66

[melifaro]

Address comments.

[melifaro]

In D37902#861235, @jlduran_gmail.com wrote:

Couple more nits.
New run (this time as expected):
https://cirrus-ci.com/task/5756291799842816?logs=test_examples
I'll eventually add a simple example that consumes Scapy directly (instead of doing it via a shell script).

That would be awesome!
There are quite a bunch of scapy-based tests, that are indeed mostly shell-based.

tests/examples/Makefile

5

Indeed, I forgot to make an mtree change.

tests/examples/test_examples.py

98

Yep, I forgot about the IPv4 docprefixes. Thank you!

[jlduran]

Thank you!
From my perspective, this is an excellent intro to testing.

tests/examples/Makefile

5

But, besides that, I believe ATF_TESTS_PYTEST does not create the directory. If we use a C- or a shell-based ATF_TEST, it'll work. I see a message about it not stripping the .py extension, but I need to investigate more.
See:
https://cirrus-ci.com/task/5633606631030784?logs=setup#L17
Again, not directly relevant to this revision.

tests/examples/test_examples.py

54

May I ask, why do you keep this line commented out?
CirrusCI has a bad habit (and grudgingly I) of running the tests as root.
See:
https://cirrus-ci.com/task/4564450993242112?logs=test_examples_report#L87

[asomers]

This looks great. I especially like the ability to parameterize test cases, which isn't easy with atf-sh-api or atf-c-api. What about cleanup? I think atf-python tests need to do that differently, right? It would be good to add an example for that.

[melifaro]

In D37902#861452, @asomers wrote:

This looks great. I especially like the ability to parameterize test cases, which isn't easy with atf-sh-api or atf-c-api. What about cleanup? I think atf-python tests need to do that differently, right? It would be good to add an example for that.

Thank you!
Yes, the cleanup is different from the native pytest approach. The initial driving usecase was to enable cheap VNET-run tests, where the cleanup procedures are fixed - you need to remove the jail & interfaces. That part is done automatically by the framework.
I'll add a hook that'll check if the test_name_cleanup() method exists and calls it to perform the cleanup, along with the example here. Does it look reasonable to you?

tests/examples/Makefile

5

My current (limited) understanding is the following:

1. testdir hierarchy is created during the installworld target somewhere here
2. I'm not sure if auto-creation works. When I delete /usr/tests and try to do make -C tests clean all install I see a lot of similar problems with all type of tests

tests/examples/test_examples.py

54

Sure, two reasons.
unprivileged is a "special" user that kyua translates to the value of unprivileged_user from the config file.

1. We don't have it set outside of ci ATM (which can/should be fixed).
2. For VNET tests the framework should require the root user BUT fetch the unprivileged_user and switch to it before passing the control to the test method. This is not implemented yet.

Guess it's time to do to just that - as it turns out I need this one for netlink testing.

So, I'd say it's worth keeping as is untils at least one of two issues get resoled. Thoughts?

[asomers]

A "test_case_cleanup" method would work. But it would be more Pythonic if it were a teardown method. Would that be possible?

tests/examples/test_examples.py

54

No. If the "unprivileged_user" isn't set in /usr/local/etc/kyua.conf, then Kyua should skip the test. So I still think you should uncomment that line.

[jlduran]

In D37902#861460, @melifaro wrote:

In D37902#861452, @asomers wrote:

This looks great. I especially like the ability to parameterize test cases, which isn't easy with atf-sh-api or atf-c-api. What about cleanup? I think atf-python tests need to do that differently, right? It would be good to add an example for that.

Thank you!
Yes, the cleanup is different from the native pytest approach. The initial driving usecase was to enable cheap VNET-run tests, where the cleanup procedures are fixed - you need to remove the jail & interfaces. That part is done automatically by the framework.

While on this topic, the cleanup for the native pytest approach is done the "old" way. I have encountered a few races, where an epair interface remains after a massive test failure. In other words, I was contemplating the possibility of adapting 80fc25025ffcb0d369fc0b6d4d272ad6fd3f53c3, which destroys the epair assigned to the vnet from inside the jail. Obviously, this has nothing to do with this introductory tutorial.

tests/examples/Makefile

5

My understanding is even more limited! I was just trying to take a shortcut.
I may investigate more on this subject eventually, thanks for the pointer.

tests/examples/test_examples.py

54

Yes, I see. Thanks for the detailed explanation. I wasn't aware of all those details.
So, for No. 2, maybe passing allow.suser = 0 to the cmd when create_vnet()?

[melifaro]

In D37902#861466, @jlduran_gmail.com wrote:

In D37902#861460, @melifaro wrote:

In D37902#861452, @asomers wrote:

This looks great. I especially like the ability to parameterize test cases, which isn't easy with atf-sh-api or atf-c-api. What about cleanup? I think atf-python tests need to do that differently, right? It would be good to add an example for that.

Thank you!
Yes, the cleanup is different from the native pytest approach. The initial driving usecase was to enable cheap VNET-run tests, where the cleanup procedures are fixed - you need to remove the jail & interfaces. That part is done automatically by the framework.

While on this topic, the cleanup for the native pytest approach is done the "old" way. I have encountered a few races, where an epair interface remains after a massive test failure. In other words, I was contemplating the possibility of adapting 80fc25025ffcb0d369fc0b6d4d272ad6fd3f53c3, which destroys the epair assigned to the vnet from inside the jail. Obviously, this has nothing to do with this introductory tutorial.

Yep, thanks for reminding! There is a known problem resulting from a corner case in the kernel delayed object reclamation model. I promised glebius@ to alter the mechanism as a workaround, but haven't done it yet. That's something that indeed needs to happen. I'll fix this in a day or two.

[melifaro]

tests/examples/test_examples.py

54

I looked at the code to refresh that part.
So kyua runner passes the parameters in -vKEY=VALUE pairs. They are parsed by the atf-wrapper and converted to _ATF_VAR_KEY=VALUE env variables.
I'm currently thinking of implementing the following logic:

If @pytest.mark.require_user("unprivileged") is set,

1. add require.config=unprivileged-user and
2. if the framework requires root to execute (some class property, set to True for VNETs)

2.1 pass require.user="root"
2.2 fetch unprivileged-user value and switch to it before calling the actual test method.

Does it sound reasonable?

[melifaro]

tests/examples/test_examples.py

54

Re uncommenting - will do. Let me come up with the unprivileged-user fix first.

[melifaro]

Uncomment unprivileged-user
Add example with the test cleanup

[melifaro]

In D37902#861463, @asomers wrote:

A "test_case_cleanup" method would work. But it would be more Pythonic if it were a teardown method. Would that be possible?

Whoops. Sorry, I missed that comment. It landed as "cleanup_test_case" to mimic the generic "cleanup" method that already exists. That method was modelled after kuya.
I tried to make the framework abstracted from kuya/atf, but that part indeed slipped through the cracks. I'll come up with a separate renaming review next week if the time permits.
Meanwhile the per-test cleanups landed, with an example provided in this diff. Hope it does work for you :-)

tests/examples/test_examples.py

54

Uncommented. I've published D37923 to address the dropping-privileges issue.

[melifaro]

I'm going to commit it as is for now and address the remaining issue in the follow-up commits.

[melifaro]

In D37902#861660, @melifaro wrote:

In D37902#861463, @asomers wrote:

A "test_case_cleanup" method would work. But it would be more Pythonic if it were a teardown method. Would that be possible?

Whoops. Sorry, I missed that comment. It landed as "cleanup_test_case" to mimic the generic "cleanup" method that already exists. That method was modelled after kuya.
I tried to make the framework abstracted from kuya/atf, but that part indeed slipped through the cracks. I'll come up with a separate renaming review next week if the time permits.
Meanwhile the per-test cleanups landed, with an example provided in this diff. Hope it does work for you :-)

Raised D37971 to rename the method.