Integration of pytest

[polybassa]

This is just a PoC. A *.uts file will be transformed to a pytest like file. The exported file usually needs manual modifications, but this exporter can be a first step towards pytest

[codecov[bot]]

Codecov Report

Merging #2550 (0dc7496) into master (2f07f21) will increase coverage by 2.91%. The diff coverage is n/a.

:exclamation: Current head 0dc7496 differs from pull request most recent head 8e24fdf. Consider uploading reports for the commit 8e24fdf to get more accurate results

@@            Coverage Diff             @@
##           master    #2550      +/-   ##
==========================================
+ Coverage   84.57%   87.49%   +2.91%     
==========================================
  Files         296      267      -29     
  Lines       62137    55824    -6313     
==========================================
- Hits        52555    48845    -3710     
+ Misses       9582     6979    -2603     

Impacted Files	Coverage Δ
scapy/contrib/automotive/gm/gmlan_logging.py	73.33% <0.00%> (-26.67%)	:arrow_down:
scapy/layers/l2tp.py	88.88% <0.00%> (-11.12%)	:arrow_down:
scapy/themes.py	80.98% <0.00%> (-10.19%)	:arrow_down:
scapy/modules/p0f.py	64.72% <0.00%> (-9.93%)	:arrow_down:
scapy/contrib/automotive/scanner/test_case.py	76.00% <0.00%> (-8.89%)	:arrow_down:
scapy/layers/tls/crypto/cipher_aead.py	81.30% <0.00%> (-6.53%)	:arrow_down:
scapy/layers/tls/crypto/groups.py	87.14% <0.00%> (-5.82%)	:arrow_down:
scapy/autorun.py	78.23% <0.00%> (-5.21%)	:arrow_down:
scapy/contrib/geneve.py	92.10% <0.00%> (-5.20%)	:arrow_down:
scapy/layers/tls/session.py	83.78% <0.00%> (-4.10%)	:arrow_down:
... and 229 more

[gpotter2]

Please don't merge this as is. This is a great script but I'd like to see how well it works (a fully converted file) before doing anything.

I'm still unsure whether it's best to move to pytest because of how massive the work will probably end up being..

[polybassa]

This is absolutely not ready for merge. Sorry for not mentioning this.

The produced files will all need manual rework.

This is just a starting point. The use of pytest needs to be evaluated for the tests, in my opinion.

Viele Grüße

Nils Weiß

On March 26, 2020 5:21:07 PM GMT+01:00, Gabriel [email protected] wrote:

Please don't merge this as is. This is a great script but I'd like to see how well it works (a fully converted file) before doing anything.

I'm still unsure whether it's best to move to pytest because of how massive the work will probably end up being..

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/secdev/scapy/pull/2550#issuecomment-604526529

[polybassa]

I've added pytest to tox (just for PoC, not added everywhere). What about supporting pytest and UTscapy in parallel?

[polybassa]

@gpotter2 @guedou @p-l- I would like to start a discussion. I've made a small PoC to enable pytest for the unit tests. I respect the individual configurations by parsing for the argument "-K" A simple dump function in UTscapy allows to transfer a *.uts file to a pytest file. Manual fixing is required here, nevertheless this script should save some time if one wants to port tests. I've tested codecov if the combination of tests work corretly, if pytest and uts files for the same unit differ.

Would it make sense to open the unit test system to allow contributors to write tests in *.uts or pytest?

[guedou]

This is a really cool PR. It its definitely worth finishing it and merging. If @gpotter2 and @p-l- are OK, I think that we should move to pytest and leave UTScapy behind us. After merging this PR, the next step will be to mandate that new unit tests are written with pytest while we migrate all tests.

Did you try concurrent testing? I will be interested to know if moving to pytest will also make the testing time shorter.

[polybassa]

Thanks for your feedback. I will continue my work on this PR. I will probably start to port regression.uts to pytest, to see if this concept holds. I didn't tested concurrent execution, yet. Also my tests on CAN will probably fail if they were executed concurrently. But I will test this, after I ported some tests from regression.uts.

[gpotter2]

I agree with @guedou that this is very cool, and it would be great to be able to migrate to pytest, but we must make sure everything works before doing that. Thé UTscapy file is filled with tons of extra code that would need to be integrated somehow to our test process.

[polybassa]

I totally agree, the UTscapy file is pretty much filled with stuff. I did a rough analysis of the current functionality.. in my opinion, some function might not be useful, some functions will be available in pytest. Most important is the tag system, which I could "somehow" port to pytest. But I will continue on that PoC, than we can see, where leads to.

[guedou]

Before converting moving to pytest, I think that it will make sense to split regression.uts into smaller files that respect scapy/ naming convention i.e. test/config.uts test/layers/inet6.uts test/packet.uts ...

First, it will make your life easier when adding or looking for a unit test, then we will be able to convert to pytest file by file with several simple PR instead of a huge one.

[p-l-]

That is really great. Thanks a lot for this work.

I agree with @guedou that we may want so split regression.uts.

[polybassa]

Hi, I've updated this PR and would like to open it for your review.

I made some first ports to pytest. I used the Co-Author tag to preserve the original authors (git blame filename --porcelain | grep "^author " | sort | uniq was used to identify the original authors).

I would suggest the enforcement of flake8 for unit tests. The possibility of pytest would help us to have much cleaner unit tests, in my opinion.

[polybassa]

And another question. Should there be a copyright notice on unit tests?

[polybassa]

I will collect all ports of *.uts files to test_*.py files here in individual commits

[polybassa]

Absolutely. Thanks for this idea.

On October 6, 2020 6:09:13 PM GMT+02:00, Gabriel [email protected] wrote:

@gpotter2 commented on this pull request.

@@ -0,0 +1,1940 @@ +import pytest +from scapy.all import *

+# DUID_LLT basic instantiation +def test_DUIDLLTbasicinstantiation():

Don't you think it would be cleaner to have the comment as a docstring rather than a comment?

-- You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: https://github.com/secdev/scapy/pull/2550#pullrequestreview-503132488

[polybassa]

I've added your suggestion

[polybassa]

Does it make sense to continue porting individual unit test, as I did already in this PR? I'm a little bit afraid, that changes to uts files that are already ported in this PR get lost.

[gpotter2]

The best thing would be that you wait for all the files to be split before wasting time fixing those files. We need to have this done file by file otherwise the history will be a mess.

[gpotter2]

Just to be clear, this is incredible work. it's just a bit too early

[polybassa]

Ok, perfect. Thanks.

I'll try to assist with splitting regression.uts

On October 14, 2020 8:05:47 PM GMT+02:00, Gabriel [email protected] wrote:

Just to be clear, this is incredible work. it's just a bit too early

-- You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: https://github.com/secdev/scapy/pull/2550#issuecomment-708569851

[guedou]

I made a quick and dirty script that automates the conversion and maintains the the unit tests authorship in the commits. The commits for scapy/layers are ready but I am not pushing them to avoid conflicts.

I will try to open an issue soon in order to synchronize.

[polybassa]

Alright. Than I just wait :-)

[guedou]

@polybassa I don't know what is the best way to sync on this topic. Could you do a first PR to split VXLAN tests out of regression.uts using suggestions from https://github.com/secdev/scapy/pull/2755#pullrequestreview-464408077 and the PR that I recently made?

[polybassa]

Of course. I suppose the following:

1. I make a minimal PR just to add the configurations for pytest
2. I make one PR for the migration of VXLAN utscapy to pytest

Does this align with your intention?

[guedou]

I meant moving the vxlan tests out of regression.uts as I recently did for other protocols. I still consider that it is too early to use pytest and that we need to split regression.uts first.

An alternative to pytest could be to the cw the uts in parallel

[polybassa]

Ah alright.. I misunderstood. I will look into vxlan tests and try to split them.

Viele Grüße

Nils Weiß

On October 24, 2020 11:02:11 AM GMT+02:00, Guillaume Valadon [email protected] wrote:

I meant moving the vxlan tests out of regression.uts as I recently did for other protocols. I still consider that it is too early to use pytest and that we need to split regression.uts first.

An alternative to pytest could be to the cw the uts in parallel

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/secdev/scapy/pull/2550#issuecomment-715884461

[polybassa]

@guedou See #2913

[polybassa]

@guedou should I continue with extracting layers and "modules" from regression.uts? I saw

• Scapy version
• Packetlist
• MPLS
• Random
• Fields
• ARP
• inet and some more.

[guedou]

We have an issue with Travis right now: we ran out free credits to launch the unit tests. I think that we need to focus on mandatory fixes first =/

I am taking care of inet.py and l2.py.

What is the issue with MPLS? It seems that that test are located in the correct location.

[polybassa]

Ok, so I will not do further splits of regression, until I get a sign from you. Regarding MPLS, I found this section in regression.uts: https://github.com/secdev/scapy/blob/c09354817d268c31674549082b9305ece6b1ad66/test/regression.uts#L4851-L4916