Skip to content

Conversation

@mattdibi
Copy link
Contributor

@mattdibi mattdibi commented Mar 6, 2024

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

While working on a strictly timed test for IPv6 firewall we noticed that the neighsol function was not respecting the given timeout. Looking at the code it was apparent that the timeout variable was not actually used in the code. This PR fixes that.

For unit tests I might require some guidance...

@mattdibi mattdibi changed the title Fix neighsol timeout not actually used Fix layers.inet6.neighsol timeout not actually being used Mar 6, 2024
@codecov
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

Merging #4310 (99a0af5) into master (42b98a2) will decrease coverage by 0.88%.
The diff coverage is 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4310      +/-   ##
==========================================
- Coverage   81.37%   80.49%   -0.88%     
==========================================
  Files         350      350              
  Lines       82282    82282              
==========================================
- Hits        66954    66231     -723     
- Misses      15328    16051     +723     
Files Coverage Δ
scapy/layers/inet6.py 88.67% <100.00%> (ø)

... and 29 files with indirect coverage changes

@guedou
Copy link
Member

guedou commented Mar 7, 2024

Good catch! Thanks for this PR.

@guedou guedou enabled auto-merge (squash) March 7, 2024 06:17
@guedou guedou merged commit a05013c into secdev:master Mar 7, 2024
@mattdibi mattdibi deleted the fix/neighsol_timeout branch March 7, 2024 07:14
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