test(hypervisors): add unit tests for utils.go helpers by parthdagia05 · Pull Request #584 · urunc-dev/urunc

parthdagia05 · 2026-04-28T22:49:49Z

Description

I have added unit tests for pkg/unikontainers/hypervisors/utils.go, which currently has no test coverage.

The tests use testify/assert and a table-driven, t.Parallel-friendly style consistent with the existing vmm_test.go in the same package.

Functions covered:

cpuArch - verified against runtime.GOARCH so the same test works on both amd64 and aarch64 builds.
appendNonEmpty - six cases covering empty/non-empty body, prefix and value combinations.
bytesToMiB - exact MiB boundary, sub-MiB truncation, large values.
bytesToMB - same coverage shape with the decimal MB constant.
BytesToStringMB - the three logical branches: zero argument falls back to DefaultMemory, sub-MB value falls back to DefaultMemory, and a normal value passes through.

I also added one extra test (TestBytesToMiBVsMBDistinction) that asserts the binary vs decimal megabyte constants are not interchangeable. Its goal is to catch a refactor that accidentally swaps the two unit helpers - they have the same signature so the compiler won't notice.

killProcess is intentionally not covered in this PR. It requires spawning a real subprocess, and several of its branches cannot be reproduced deterministically without privileged operations or refactoring the function to inject the signaller. Happy to address it in a follow-up if useful.

Related issues

Refs #96

How was this tested?

go test -v ./pkg/unikontainers/hypervisors/... - 6 tests, 23 subtests, all pass.
go test -count=1 ./... (excluding tests/e2e which needs Docker/QEMU/crictl) - every previously-passing package still passes.
gofmt -l pkg/unikontainers/hypervisors/utils_test.go - clean.
go vet ./pkg/unikontainers/hypervisors/... - clean.
golangci-lint run ./pkg/unikontainers/hypervisors/... (using the project's .golangci.yml v2 config) - 0 issues.
make lint - passes locally.

LLM usage

Anthropic Claude Opus 4.6 was used to assist with planning the test scope, identifying edge cases for each helper. All code was reviewed and tested by me before submission.

Checklist

I have read the contribution guide.
The linter passes locally (make lint).
The e2e tests of at least one tool pass locally (make test_ctr, make test_nerdctl, make test_docker, make test_crictl).
If LLMs were used: I have read the llm policy.

netlify · 2026-04-28T22:49:55Z

✅ Deploy Preview for urunc canceled.

Name	Link
🔨 Latest commit	`04c6f6d`
🔍 Latest deploy log	https://app.netlify.com/projects/urunc/deploys/6a06037a34840c0008e82ce3

cmainas · 2026-04-29T05:25:32Z

Hello @parthdagia05 ,

thank you for this PR> Please do not overwrite the PR template.

parthdagia05 · 2026-04-29T05:53:36Z

Hello @parthdagia05 ,

thank you for this PR> Please do not overwrite the PR template.

Sure, made the changes according to the PR template could you take a look again

cmainas

Hello @parthdagia05 ,

thank you for updating the description.

The functions cpuArch, appenNonEmpty and BytesToStringMB seem useless and we need to remove them (not in this PR). So let's drop their testing.
Please replace in and want with input and output / expected respectively. Or use some better names than in and want.

cmainas · 2026-05-04T06:40:01Z

+// TestBytesToMiBVsMBDistinction guards against a refactor that swaps the two
+// constants — 1 MB (10^6) is strictly less than 1 MiB (2^20), so a value of
+// exactly 1 MB must round to 0 MiB while reporting 1 MB.
+func TestBytesToMiBVsMBDistinction(t *testing.T) {


I do not really understand this unit test.

I do not really understand this unit test.

Ensures the binary unit and decimal-unit variants stay distinct a copy paste or refactor that collapses them into one would silently produce wrong values. Uses 1,000,000 bytes as input, where binary and decimal results must diverge.

I do not really agree with such a test. The other unit tests already cover such cases.

parthdagia05 · 2026-05-04T07:25:18Z

@cmainas removed the tests for cpuArch, appenNonEmpty and BytesToStringM.

cmainas

Hello @parthdagia05 , le;ts remove the test in question.

cmainas · 2026-05-13T11:45:56Z

+// TestBytesToMiBVsMBDistinction guards against a refactor that swaps the two
+// constants — 1 MB (10^6) is strictly less than 1 MiB (2^20), so a value of
+// exactly 1 MB must round to 0 MiB while reporting 1 MB.
+func TestBytesToMiBVsMBDistinction(t *testing.T) {


I do not really agree with such a test. The other unit tests already cover such cases.

parthdagia05 · 2026-05-13T12:18:42Z

@cmainas removed the test

cmainas · 2026-05-14T08:10:42Z

Thank you @parthdagia05 ,

you will also need to add yourself here https://github.com/urunc-dev/urunc/blob/main/.github/contributors.yaml and rebase over main

parthdagia05 · 2026-05-14T09:28:54Z

done @cmainas

cmainas · 2026-05-14T09:37:22Z

Hello @parthdagia05 ,

can you also squash the first three commits? They do not add any value as single commits and they all related to a specific change.

Adds table-driven tests for the two byte-conversion helpers in pkg/unikontainers/hypervisors/utils.go, using testify/assert. Each helper is covered for zero, sub-unit, exact-unit, multiple, and large-value inputs. Signed-off-by: Parth Dagia <parth.24bcs10414@sst.scaler.com>

parthdagia05 · 2026-05-14T10:30:49Z

@cmainas done

Signed-off-by: Parth Dagia <parth.24bcs10414@sst.scaler.com>

parthdagia05 · 2026-05-14T17:19:29Z

@cmainas the commitlint spellcheck flagged my name, can you once check it and rerun the workflows again

cmainas

Thank you @parthdagia05 for the new unit tests.

Adds table-driven tests for the two byte-conversion helpers in pkg/unikontainers/hypervisors/utils.go, using testify/assert. Each helper is covered for zero, sub-unit, exact-unit, multiple, and large-value inputs. PR: #584 Signed-off-by: Parth Dagia <parth.24bcs10414@sst.scaler.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

PR: #584 Signed-off-by: Parth Dagia <parth.24bcs10414@sst.scaler.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

Adds table-driven tests for the two byte-conversion helpers in pkg/unikontainers/hypervisors/utils.go, using testify/assert. Each helper is covered for zero, sub-unit, exact-unit, multiple, and large-value inputs. PR: #584 Signed-off-by: Parth Dagia <parth.24bcs10414@sst.scaler.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

PR: #584 Signed-off-by: Parth Dagia <parth.24bcs10414@sst.scaler.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

cmainas reviewed May 4, 2026

View reviewed changes

cmainas reviewed May 13, 2026

View reviewed changes

parthdagia05 force-pushed the test/hypervisors-utils branch from 76d44c9 to 9046782 Compare May 14, 2026 09:27

parthdagia05 force-pushed the test/hypervisors-utils branch from 9046782 to 8acfc93 Compare May 14, 2026 09:41

cmainas added the ok-to-test label May 14, 2026

chore: add contributor entry

04c6f6d

Signed-off-by: Parth Dagia <parth.24bcs10414@sst.scaler.com>

parthdagia05 force-pushed the test/hypervisors-utils branch from 8acfc93 to 04c6f6d Compare May 14, 2026 17:16

cmainas added the takeover label May 15, 2026

urunc-bot Bot changed the base branch from main to main-pr584 May 15, 2026 08:27

cmainas approved these changes May 15, 2026

View reviewed changes

cmainas merged commit 3bd9a52 into urunc-dev:main-pr584 May 15, 2026
51 of 53 checks passed

Conversation

parthdagia05 commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

How was this tested?

LLM usage

Checklist

Uh oh!

netlify Bot commented Apr 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for urunc canceled.

Uh oh!

cmainas commented Apr 29, 2026

Uh oh!

parthdagia05 commented Apr 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmainas left a comment

Choose a reason for hiding this comment

Uh oh!

cmainas May 4, 2026

Choose a reason for hiding this comment

Uh oh!

parthdagia05 May 4, 2026

Choose a reason for hiding this comment

Uh oh!

cmainas May 13, 2026

Choose a reason for hiding this comment

Uh oh!

parthdagia05 commented May 4, 2026

Uh oh!

cmainas left a comment

Choose a reason for hiding this comment

Uh oh!

cmainas May 13, 2026

Choose a reason for hiding this comment

Uh oh!

parthdagia05 commented May 13, 2026

Uh oh!

cmainas commented May 14, 2026

Uh oh!

parthdagia05 commented May 14, 2026

Uh oh!

cmainas commented May 14, 2026

Uh oh!

parthdagia05 commented May 14, 2026

Uh oh!

parthdagia05 commented May 14, 2026

Uh oh!

cmainas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

parthdagia05 commented Apr 28, 2026 •

edited

Loading

netlify Bot commented Apr 28, 2026 •

edited

Loading

parthdagia05 commented Apr 29, 2026 •

edited

Loading