Skip to content

chore(test): add tests to network utils and validations #2228

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

Merged
merged 2 commits into from
Jun 11, 2025

Conversation

JGAntunes
Copy link
Member

@JGAntunes JGAntunes commented Jun 2, 2025

What this PR does / why we need it:

Follow up to #2187

See:

We created an interface to allow us to mock and test interactions with the net package.

Which issue(s) this PR fixes:

https://app.shortcut.com/replicated/story/124371/add-tests-to-the-network-methods

Does this PR require a test?

These are tests actually 🙌

Does this PR require a release note?

NONE

Does this PR require documentation?

NONE

@JGAntunes JGAntunes requested a review from sgalsaleh June 2, 2025 14:05
@JGAntunes JGAntunes self-assigned this Jun 2, 2025
Comment on lines -17 to -19
if ipnet.IP.To4() == nil {
return "", fmt.Errorf("interface %s has no ipv4 addresses", networkInterface)
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This isn't required and will be unreachable since FirstValidIPNet will take care of filtering non ipv4 interfaces, see:

Copy link

github-actions bot commented Jun 2, 2025

This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID.

Online Installer:

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci/appver-dev-bbd1502" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Airgap Installer (may take a few minutes before the airgap bundle is built):

curl "https://staging.replicated.app/embedded/embedded-cluster-smoke-test-staging-app/ci-airgap/appver-dev-bbd1502?airgap=true" -H "Authorization: $EC_SMOKE_TEST_LICENSE_ID" -o embedded-cluster-smoke-test-staging-app-ci.tgz

Happy debugging!

@JGAntunes JGAntunes force-pushed the chore/test-network branch from 16632a0 to bbd1502 Compare June 11, 2025 08:32
@JGAntunes JGAntunes requested a review from emosbaugh June 11, 2025 14:40
@JGAntunes JGAntunes enabled auto-merge (squash) June 11, 2025 16:09
@sgalsaleh
Copy link
Member

Merging as the failing test is flaky.

@sgalsaleh sgalsaleh disabled auto-merge June 11, 2025 16:10
@sgalsaleh sgalsaleh merged commit 9eec604 into main Jun 11, 2025
126 of 129 checks passed
@sgalsaleh sgalsaleh deleted the chore/test-network branch June 11, 2025 16:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants