Skip to content
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

Fix some tests to use udp.PickPort() #1302

Merged
merged 1 commit into from Sep 30, 2021

Conversation

yuhan6665
Copy link
Contributor

@codecov-commenter
Copy link

Codecov Report

Merging #1302 (418b499) into master (deb9d08) will decrease coverage by 0.23%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1302      +/-   ##
==========================================
- Coverage   44.64%   44.40%   -0.24%     
==========================================
  Files         485      476       -9     
  Lines       29534    29435      -99     
==========================================
- Hits        13184    13072     -112     
- Misses      14945    14949       +4     
- Partials     1405     1414       +9     
Impacted Files Coverage Δ
transport/internet/udp/dispatcher.go 64.76% <0.00%> (-12.39%) ⬇️
common/buf/reader.go 82.35% <0.00%> (-5.89%) ⬇️
testing/servers/tcp/tcp.go 81.81% <0.00%> (-5.46%) ⬇️
transport/internet/udp/hub.go 73.61% <0.00%> (-4.17%) ⬇️
proxy/freedom/freedom.go 45.00% <0.00%> (-4.00%) ⬇️
transport/internet/system_listener.go 50.00% <0.00%> (-4.00%) ⬇️
transport/internet/kcp/dialer.go 80.39% <0.00%> (-3.93%) ⬇️
testing/scenarios/common.go 70.68% <0.00%> (-3.45%) ⬇️
transport/internet/kcp/receiving.go 97.31% <0.00%> (-2.69%) ⬇️
transport/internet/quic/hub.go 72.00% <0.00%> (-2.67%) ⬇️
... and 14 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update deb9d08...418b499. Read the comment docs.

@yuhan6665
Copy link
Contributor Author

This check fail:
https://github.com/v2fly/v2ray-core/pull/1302/checks?check_run_id=3726781714#step:5:2727
is not the original issue but a different one. This one seems more complicated which I will work on the fix later ;)

Copy link
Contributor

@xiaokangwang xiaokangwang left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution.

@yuhan6665 did't describe the root cause of the issue being fixed. Based on the content of the pull request I have a educated guess of why this fix is needed.

In the previous version of the test code, when attempting to listen for a UDP socket, the port availability check check for TCP port instead of UDP port. This make it possible to select a UDP port that is unavailable.

This pull request is ready to be merged.

@yuhan6665
Copy link
Contributor Author

Thanks for the review!
I will try to improve my English description in the future ;)

@xiaokangwang xiaokangwang merged commit 9458a1a into v2fly:master Sep 30, 2021
xiaokangwang pushed a commit that referenced this pull request Sep 30, 2021
(cherry picked from commit 9458a1a, modified by committer)

In the previous version of the test code, when attempting to listen for a UDP socket, the port availability check check for TCP port instead of UDP port. This make it possible to select a UDP port that is unavailable.

See Also: #1302
@yuhan6665 yuhan6665 deleted the fix-udp-test branch October 3, 2021 15:43
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.

None yet

3 participants