Skip to content

Provide a diff feature that refine packet comparison #4779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

alxroyer-thales
Copy link

@alxroyer-thales alxroyer-thales commented Jun 26, 2025

Draft PR for #4739.

Initiated as a draft PR for preliminary discussions before an official delivery (not squashed yet).

To be discussed:

  • Chaged currently based on v2.6.1 => shall I align on master? or useful to deliver for a version before?

Checklist:

  • If you are new to Scapy: I have checked CONTRIBUTING.md (esp. section submitting-pull-requests)
  • I squashed commits belonging together
  • I added unit tests or explained why they are not relevant
  • I executed the regression tests (using cd test && ./run_tests or tox)
  • If the PR is still not finished, please create a Draft Pull Request

- `ApproximateField` class added in 'fields.py'.
- `PacketCmp` class added in a new 'diff.py' module, exported in 'all.py'.
- 'test/diff.uts' unit test added.
@gpotter2
Copy link
Member

Could you show a demo of what it does in action? Thanks.

@@ -3,6 +3,7 @@
# See https://scapy.net/ for more information
# Copyright (C) Philippe Biondi <[email protected]>
# Copyright (C) Michael Farrell <[email protected]>
# Copyright (C) 2025 Thales
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My personal opinion : this one is unecessary considering the changes in the file.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've set this line following our legal department recommendations.
Let me check with them.

@alxroyer-thales
Copy link
Author

Could you show a demo of what it does in action? Thanks.

Sure. It would be worth adding faithful documentation in the end if you validate this proposal.

To reply quickly, here are extracts of outputs from the 'diff.uts' unit test I've proposed.

Comparison of packet with exact match

###(005)=[passed] 005) Compare a1 with a2 => PacketCmp.compare() returns ``True``.

>>> cmp = PacketCmp(a1, a2)
>>> assert cmp.compare(log_success_level=logging.INFO) is True
A.k: 1 (compared) == 1 (expected)
A.t: 1751014222 (compared) == 1751014222 (expected)


###(006)=[passed] 006) The comparison instance holds 2 successful exact comparisons, and no errors.

>>> print(f"diffs: {cmp.diffs!r}")
diffs: [<PacketCmp.Diff 'A.k: 1 (compared) == 1 (expected)'>, <PacketCmp.Diff 'A.t: 1751014222 (compared) == 1751014222 (expected)'>]
>>> assert len(cmp.diffs) == 2
>>> check_diff(cmp.diffs[0], "k", error=False, approx=False)
>>> check_diff(cmp.diffs[1], "t", error=False, approx=False)
>>> print(f"errors: {cmp.errors!r}")
errors: []
>>> assert len(cmp.errors) == 0

Memo: The k field is a simple ByteField, and t is an ApproximateField (see step 002).

The lines after the .compare() (step 005) are loggings activated by the log_success_level=logging.INFO configuration.

The PacketCmp object holds two lists in the end:

  • diffs: All PacketCmp.Diff objects, successful comparisons included.
  • errors: diffs` filtered on errors.

Successful comparison with approximation

###(009)=[passed] 009) Compare a1 with a2 => PacketCmp.compare() returns ``True``.

>>> cmp = PacketCmp(a1, a2)
>>> assert cmp.compare(log_success_level=logging.INFO) is True
A.t: 1751014222 (compared) ~= (delta: 1.0 <= tolerance: 2.0) 1751014221 (expected) -- comparison restarted
A.k: 1 (compared) == 1 (expected)
A.t: 1751014222 (compared) == 1751014222 (expected)


###(010)=[passed] 010) The comparison instance holds 3 diffs, the 1st being the approximation that restarted the comparison, and no errors.

>>> print(f"diffs: {cmp.diffs!r}")
diffs: [<PacketCmp.Diff 'A.t: 1751014222 (compared) ~= (delta: 1.0 <= tolerance: 2.0) 1751014221 (expected) -- comparison restarted'>, <PacketCmp.Diff 'A.k: 1 (compared) == 1 (expected)'>, <PacketCmp.Diff 'A.t: 1751014222 (compared) == 1751014222 (expected)'>]
>>> assert len(cmp.diffs) == 3
>>> check_diff(cmp.diffs[0], "t", error=False, approx=True)
>>> check_diff(cmp.diffs[1], "k", error=False, approx=False)
>>> check_diff(cmp.diffs[2], "t", error=False, approx=False)
>>> print(f"errors: {cmp.errors!r}")
errors: []
>>> assert len(cmp.errors) == 0

Comparison failure with aproximate field out of tolerance

###(013)=[passed] 013) Compare a1 with a2 => PacketCmp.compare() returns ``False``.

>>> cmp = PacketCmp(a1, a2)
>>> assert cmp.compare(log_success_level=logging.INFO) is False
A.k: 1 (compared) == 1 (expected)
A.t: 1751014222 (compared) != (delta: 3.0 > tolerance: 2.0) 1751014219 (expected) -- Mismatching values


###(014)=[passed] 014) The comparison instance holds 2 diffs, 1 error among them.

>>> print(f"diffs: {cmp.diffs!r}")
diffs: [<PacketCmp.Diff 'A.k: 1 (compared) == 1 (expected)'>, <PacketCmp.Diff 'A.t: 1751014222 (compared) != (delta: 3.0 > tolerance: 2.0) 1751014219 (expected) -- Mismatching values'>]
>>> assert len(cmp.diffs) == 2
>>> check_diff(cmp.diffs[0], "k", error=False, approx=False)
>>> check_diff(cmp.diffs[1], "t", error=True, approx=True)
>>> print(f"errors: {cmp.errors!r}")
errors: [<PacketCmp.Diff 'A.t: 1751014222 (compared) != (delta: 3.0 > tolerance: 2.0) 1751014219 (expected) -- Mismatching values'>]
>>> check_diff(cmp.errors[0], "t", error=True, approx=True)

Copy link

codecov bot commented Jul 1, 2025

Codecov Report

Attention: Patch coverage is 78.22581% with 54 lines in your changes missing coverage. Please review.

Project coverage is 81.21%. Comparing base (8e08cbf) to head (dcf0b85).
Report is 114 commits behind head on master.

Files with missing lines Patch % Lines
scapy/diff.py 77.29% 52 Missing ⚠️
scapy/fields.py 88.88% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4779      +/-   ##
==========================================
- Coverage   81.62%   81.21%   -0.42%     
==========================================
  Files         358      364       +6     
  Lines       85652    88652    +3000     
==========================================
+ Hits        69915    72000    +2085     
- Misses      15737    16652     +915     
Files with missing lines Coverage Δ
scapy/all.py 100.00% <100.00%> (ø)
scapy/fields.py 92.79% <88.88%> (+0.02%) ⬆️
scapy/diff.py 77.29% <77.29%> (ø)

... and 80 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants