Skip to content

Commit 808f641

Browse files
committed
fix: address review feedback for disk space check
- Add overflow guard for Bavail * Bsize multiplication (cap at MaxInt64) - Handle negative values in formatBytes (return "0 B") - Add edge case tests for Check (empty path, root dir, deeply nested) - Add unit tests for estimateBuildSize (files, dirs, missing, empty) - Fix copyright year to 2025 in new files Signed-off-by: Zhao Chen <winters.zc@antgroup.com>
1 parent 007f722 commit 808f641

3 files changed

Lines changed: 111 additions & 5 deletions

File tree

pkg/backend/build_test.go

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,14 @@
1717
package backend
1818

1919
import (
20+
"os"
21+
"path/filepath"
2022
"testing"
2123

24+
"github.com/stretchr/testify/assert"
25+
2226
"github.com/modelpack/modctl/pkg/config"
2327
"github.com/modelpack/modctl/test/mocks/modelfile"
24-
25-
"github.com/stretchr/testify/assert"
2628
)
2729

2830
func TestGetProcessors(t *testing.T) {
@@ -41,3 +43,69 @@ func TestGetProcessors(t *testing.T) {
4143
assert.Equal(t, "code", processors[2].Name())
4244
assert.Equal(t, "doc", processors[3].Name())
4345
}
46+
47+
func TestEstimateBuildSize(t *testing.T) {
48+
t.Run("single files", func(t *testing.T) {
49+
workDir := t.TempDir()
50+
51+
// Create test files with known sizes.
52+
assert.NoError(t, os.WriteFile(filepath.Join(workDir, "model.bin"), make([]byte, 1024), 0644))
53+
assert.NoError(t, os.WriteFile(filepath.Join(workDir, "config.json"), make([]byte, 256), 0644))
54+
55+
mf := &modelfile.Modelfile{}
56+
mf.On("GetConfigs").Return([]string{"config.json"})
57+
mf.On("GetModels").Return([]string{"model.bin"})
58+
mf.On("GetCodes").Return([]string{})
59+
mf.On("GetDocs").Return([]string{})
60+
61+
size := estimateBuildSize(workDir, mf)
62+
assert.Equal(t, int64(1280), size)
63+
})
64+
65+
t.Run("directory entry", func(t *testing.T) {
66+
workDir := t.TempDir()
67+
68+
// Create a subdirectory with files.
69+
subDir := filepath.Join(workDir, "models")
70+
assert.NoError(t, os.MkdirAll(subDir, 0755))
71+
assert.NoError(t, os.WriteFile(filepath.Join(subDir, "a.bin"), make([]byte, 512), 0644))
72+
assert.NoError(t, os.WriteFile(filepath.Join(subDir, "b.bin"), make([]byte, 512), 0644))
73+
74+
mf := &modelfile.Modelfile{}
75+
mf.On("GetConfigs").Return([]string{})
76+
mf.On("GetModels").Return([]string{"models"})
77+
mf.On("GetCodes").Return([]string{})
78+
mf.On("GetDocs").Return([]string{})
79+
80+
size := estimateBuildSize(workDir, mf)
81+
assert.Equal(t, int64(1024), size)
82+
})
83+
84+
t.Run("nonexistent file is skipped", func(t *testing.T) {
85+
workDir := t.TempDir()
86+
87+
assert.NoError(t, os.WriteFile(filepath.Join(workDir, "real.bin"), make([]byte, 100), 0644))
88+
89+
mf := &modelfile.Modelfile{}
90+
mf.On("GetConfigs").Return([]string{})
91+
mf.On("GetModels").Return([]string{"real.bin", "missing.bin"})
92+
mf.On("GetCodes").Return([]string{})
93+
mf.On("GetDocs").Return([]string{})
94+
95+
size := estimateBuildSize(workDir, mf)
96+
assert.Equal(t, int64(100), size)
97+
})
98+
99+
t.Run("empty modelfile", func(t *testing.T) {
100+
workDir := t.TempDir()
101+
102+
mf := &modelfile.Modelfile{}
103+
mf.On("GetConfigs").Return([]string{})
104+
mf.On("GetModels").Return([]string{})
105+
mf.On("GetCodes").Return([]string{})
106+
mf.On("GetDocs").Return([]string{})
107+
108+
size := estimateBuildSize(workDir, mf)
109+
assert.Equal(t, int64(0), size)
110+
})
111+
}

pkg/diskspace/check.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2024 The CNAI Authors
2+
* Copyright 2025 The CNAI Authors
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -18,6 +18,7 @@ package diskspace
1818

1919
import (
2020
"fmt"
21+
"math"
2122
"os"
2223
"path/filepath"
2324

@@ -59,7 +60,16 @@ func Check(dir string, requiredBytes int64) error {
5960
}
6061

6162
// Available space for non-root users.
62-
availableBytes := int64(stat.Bavail) * int64(stat.Bsize)
63+
// Guard against overflow: on Linux Bavail is uint64, and values exceeding
64+
// math.MaxInt64 would wrap negative when cast to int64. Cap at MaxInt64.
65+
bavail := stat.Bavail
66+
bsize := uint64(stat.Bsize)
67+
var availableBytes int64
68+
if bavail > 0 && bsize > uint64(math.MaxInt64)/bavail {
69+
availableBytes = math.MaxInt64
70+
} else {
71+
availableBytes = int64(bavail * bsize)
72+
}
6373
requiredWithMargin := int64(float64(requiredBytes) * safetyMargin)
6474

6575
if availableBytes < requiredWithMargin {
@@ -74,6 +84,10 @@ func Check(dir string, requiredBytes int64) error {
7484

7585
// formatBytes formats bytes into a human-readable string.
7686
func formatBytes(bytes int64) string {
87+
if bytes < 0 {
88+
return "0 B"
89+
}
90+
7791
const (
7892
kb = 1024
7993
mb = kb * 1024

pkg/diskspace/check_test.go

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2024 The CNAI Authors
2+
* Copyright 2025 The CNAI Authors
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -62,11 +62,35 @@ func TestCheck_NonExistentDirWalksUp(t *testing.T) {
6262
}
6363
}
6464

65+
func TestCheck_EmptyPath(t *testing.T) {
66+
// Empty string should walk up to root and succeed for small size
67+
err := Check("", 1)
68+
if err != nil {
69+
t.Errorf("expected nil error for empty path with 1 byte, got: %v", err)
70+
}
71+
}
72+
73+
func TestCheck_RootDir(t *testing.T) {
74+
err := Check("/", 1)
75+
if err != nil {
76+
t.Errorf("expected nil error for root dir with 1 byte, got: %v", err)
77+
}
78+
}
79+
80+
func TestCheck_DeeplyNestedNonExistentDir(t *testing.T) {
81+
err := Check("/tmp/a/b/c/d/e/f/g/h", 1)
82+
if err != nil {
83+
t.Errorf("expected nil error for deeply nested non-existent path, got: %v", err)
84+
}
85+
}
86+
6587
func TestFormatBytes(t *testing.T) {
6688
tests := []struct {
6789
bytes int64
6890
expected string
6991
}{
92+
{-1, "0 B"},
93+
{-1024, "0 B"},
7094
{0, "0 B"},
7195
{500, "500 B"},
7296
{1024, "1.00 KB"},

0 commit comments

Comments
 (0)