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

Refactor NAT discovery unit tests to use the public Interface #3334

Open
wants to merge 2 commits into
base: devel
Choose a base branch
from

Conversation

tpantelis
Copy link
Contributor

@tpantelis tpantelis commented Mar 6, 2025

This makes them more "whitebox" and moves the files to the natdiscovery_test package to avoid leakage of test code into the production package. This will also make it a bit easier to implement IPv6.

Two of the test files, request_handle_internal_test.go and listener_internal_test.go, were not moved to the test package as they test specific internal functions which would entail making too much of the production code public.

A new function, NewWithConfig, was added that allows the unit tests to construct the natDiscovery instance with hooks to mock behavior.

Previously, the remote Endpoint tests wrote directly to the local
instance UDP channel and forwarded to the
parseAndHandleMessageFromAddress method. However this bypassed the
listener loop and its setup, losing code coverage. This commit adds
a ServerConnection interface abstraction allowing the unit tests to
only mock the UDP read/write functions and utilize the listener loop.

Signed-off-by: Tom Pantelis <[email protected]>
@submariner-bot
Copy link
Contributor

🤖 Created branch: z_pr3334/tpantelis/nat_disc
🚀 Full E2E won't run until the "ready-to-test" label is applied. I will add it automatically once the PR has 2 approvals, or you can add it manually.

@tpantelis tpantelis force-pushed the nat_disc branch 2 times, most recently from 1cdc79f to deba4e3 Compare March 6, 2025 13:19
This makes them more "whitebox" and moves the files to the
"natdiscovery_test" package to avoid leakage of test code into
the production package. This should also make a bit easier to
implement IPv6.

Two of the test files, request_handle_internal_test.go and
listener_internal_test.go, were not moved to the test package as
they test specific internal functions which would entail making
too much of the production code public.

A new function, NewWithConfig, was added that allows the unit tests to
construct the natDiscovery instance with hooks to mock behavior.

Signed-off-by: Tom Pantelis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Review
Development

Successfully merging this pull request may close these issues.

2 participants