Skip to content

Conversation

@Mysterio-17
Copy link

Add Unit Tests for Vaccel, Network Utils, and Rootfs Selection

Fixes: #96

This PR adds comprehensive unit tests for previously untested functions in the urunc codebase, improving overall test coverage.

Tests Added

pkg/unikontainers/vaccel_test.go (2 test functions, 15 test cases)

  • TestIdToGuestCID - 4 test cases for deterministic vAccel guest CID generation from container IDs
  • TestIsValidVSockAddress - 11 test cases validating vsock addresses for qemu (vsock://) and firecracker (unix://) formats

pkg/unikontainers/rootfs_test.go (3 test functions, 14 test cases)

  • TestNewRootfsResult - 4 test cases for rootfs result creation (initrd, block, 9pfs, empty)
  • TestRootfsSelector_TryInitrd - 3 test cases for annotation-based initrd path detection
  • TestRootfsSelector_ShouldMountContainerRootfs - 7 test cases for boolean parsing (true/1/false/0/invalid/empty values)

pkg/network/network_test.go (3 test functions, 7 test cases)

  • TestGetTapIndex - Tests TAP device index extraction from interface names
  • TestNewNetworkManager - 5 test cases for network manager factory pattern (static/dynamic/invalid)
  • TestEnsureEth0Exists - Tests eth0 interface existence validation

Summary

  • Total: 8 test functions covering 36+ test cases
  • Coverage: Targets previously untested utility functions in vaccel.go, rootfs.go, and network.go
  • All tests passing

Signed-off-by: Mysterio-17 <mradultiwari1708@gmail.com>
Signed-off-by: Mysterio-17 <mradultiwari1708@gmail.com>
@netlify
Copy link

netlify bot commented Jan 24, 2026

Deploy Preview for urunc ready!

Name Link
🔨 Latest commit 8e50443
🔍 Latest deploy log https://app.netlify.com/projects/urunc/deploys/697a04cdf7007a0007b8bac9
😎 Deploy Preview https://deploy-preview-395--urunc.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@cmainas
Copy link
Contributor

cmainas commented Jan 26, 2026

Hello @Mysterio-17 ,

this PR also has changes from the netkit PR. These are two different PRs and the same changes should not be present in both PRs. Please remove the netkit changes from this PR and take a look in the contribution guide

This PR should only contain unit test additions. The netkit-related changes (network_netkit_test.go, netkit support functions, and dictionary updates) have been removed to keep the PR focused on unit tests only.

Signed-off-by: Mradul Tiwari <mradultiwari1708@gmail.com>
@Mysterio-17
Copy link
Author

Hello @cmainas , I’ve removed all netkit-related changes from this PR as suggested, and it now includes only unit test additions. All tests pass successfully, and the commit message has been updated to follow the contribution guidelines.
Please let me know if you have any feedback or suggestions.

Copy link
Contributor

@cmainas cmainas left a comment

Choose a reason for hiding this comment

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

Hello @Mysterio-17 ,

thank you for this contribution. I have various comments:

- Use assert package for cleaner assertions (config_test.go style)
- Simplify redundant test cases and use constants
- Rename hypervisor to monitor in vaccel tests
- Add t.Parallel() for parallel test execution

Signed-off-by: Mradul Tiwari <mradultiwari1708@gmail.com>
@Mysterio-17
Copy link
Author

Hello @cmainas,
Thanks for the thorough review! I’ve incorporated all the feedback in the latest commit.

  • I refactored the tests to use github.com/stretchr/testify/assert for cleaner assertions and aligned them with the pattern in config_test.go. I also added t.Parallel() where appropriate to enable parallel execution.
  • I cleaned up a few redundant cases by removing duplicate invalid network type tests (keeping a single representative one), dropping TestEnsureEth0Exists, and simplifying TestNewRootfsResult down to one focused test.
  • For TestIdToGuestCID, I replaced wantMin / wantMax with constants (minCID, maxCID), removed the unused checkCID field and the determinism test, and reduced it to two clear cases that directly validate the expected return values.
  • I also updated TestIsValidVSockAddress by renaming hypervisor to monitor, adding an expectedPath check for the qemu case, and renaming “unsupported hypervisor” to “unsupported monitor” for clarity.

All tests are passing now. Please let me know if any further changes are needed.

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.

Add more unit tests

2 participants