Skip to content

Commit c8046f1

Browse files
committed
refactor(test): extract hardlink helpers to internal/testfsutil, add time log
Code review follow-ups: 1. internal/testfsutil/hardlink.go — single source of truth for HardLinkDir and CopyFile. Removes three near-identical local copies from repos/ testhelpers_test.go, mpr/factory_test.go, and executor's fixture helpers. 2. Pre-commit time guard redesign: - .test-time-log: append-only history (ISO8601 timestamp, elapsed_s, git SHA, branch) committed to git so the full run history is visible - .test-time-baseline: last successful run time, committed so the guard survives machine changes; updated on every passing run - Fail if elapsed > baseline × 1.5; print last 10 log entries on failure - Reset command printed on failure for intentional perf changes 3. Remove .test-time-baseline from .gitignore (both files are now committed). Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
1 parent a37613d commit c8046f1

7 files changed

Lines changed: 136 additions & 223 deletions

File tree

.githooks/checks/02-unit-tests.sh

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,28 @@
11
#!/bin/sh
22
# L1: unit tests — fast, no external deps, no Docker.
3-
LOG_FILE="$(git rev-parse --show-toplevel)/.test-fail.log"
3+
#
4+
# Time guard: each successful run is appended to .test-time-log and the
5+
# elapsed time is compared against .test-time-baseline. Both files are
6+
# committed so the history is visible and shared.
7+
#
8+
# .test-time-baseline — last recorded elapsed time (seconds), updated on
9+
# every pass. Fail if current > baseline * 1.5.
10+
# .test-time-log — append-only history: one line per run, format:
11+
# ISO8601 elapsed_s git_sha branch
12+
#
13+
# To reset the baseline after intentional perf change:
14+
# echo <new_seconds> > .test-time-baseline
15+
# git add .test-time-baseline .test-time-log && git commit -m "perf: reset test baseline"
16+
17+
REPO="$(git rev-parse --show-toplevel)"
18+
LOG_FILE="${REPO}/.test-fail.log"
19+
BASELINE_FILE="${REPO}/.test-time-baseline"
20+
TIMELOG_FILE="${REPO}/.test-time-log"
421
rm -f "$LOG_FILE"
522

623
echo "pre-commit: running unit tests..."
24+
25+
START=$(date +%s)
726
if ! CGO_ENABLED=0 go test -timeout 180s -p "$(nproc)" -parallel "$(nproc)" ./... > "$LOG_FILE" 2>&1; then
827
echo "" >&2
928
echo "COMMIT BLOCKED: unit tests failed." >&2
@@ -13,6 +32,34 @@ if ! CGO_ENABLED=0 go test -timeout 180s -p "$(nproc)" -parallel "$(nproc)" ./..
1332
echo "CONTEXT: LOG_FILE=${LOG_FILE}" >&2
1433
exit 1
1534
fi
16-
35+
END=$(date +%s)
36+
ELAPSED=$((END - START))
1737
rm -f "$LOG_FILE"
18-
echo "pre-commit: unit tests passed."
38+
39+
# Append to time log.
40+
SHA=$(git rev-parse --short HEAD 2>/dev/null || echo "unknown")
41+
BRANCH=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || echo "unknown")
42+
TS=$(date -u +"%Y-%m-%dT%H:%M:%SZ")
43+
printf "%s\t%ss\t%s\t%s\n" "$TS" "$ELAPSED" "$SHA" "$BRANCH" >> "$TIMELOG_FILE"
44+
45+
# Time guard: compare against last successful run.
46+
if [ -f "$BASELINE_FILE" ]; then
47+
BASELINE=$(cat "$BASELINE_FILE")
48+
THRESHOLD=$((BASELINE + BASELINE / 2)) # 150% of baseline
49+
if [ "$ELAPSED" -gt "$THRESHOLD" ]; then
50+
echo "" >&2
51+
echo "COMMIT BLOCKED: test time regression." >&2
52+
echo " Current: ${ELAPSED}s" >&2
53+
echo " Baseline: ${BASELINE}s (limit: ${THRESHOLD}s = baseline × 1.5)" >&2
54+
echo "" >&2
55+
tail -10 "$TIMELOG_FILE" | awk -F'\t' '{printf " %s\t%s\n", $1, $2}' >&2
56+
echo "" >&2
57+
echo "To reset: echo ${ELAPSED} > .test-time-baseline" >&2
58+
exit 1
59+
fi
60+
fi
61+
62+
# Update baseline to the current run so the guard tracks recent history.
63+
echo "$ELAPSED" > "$BASELINE_FILE"
64+
65+
echo "pre-commit: unit tests passed (${ELAPSED}s)."

.test-time-baseline

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
60

.test-time-log

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
2026-05-27T19:55:24Z inits a37613d4 pr/mpr-pack-expr-testdata

internal/testfsutil/hardlink.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// SPDX-License-Identifier: Apache-2.0
2+
3+
// Package testfsutil provides filesystem helpers for test fixture isolation.
4+
package testfsutil
5+
6+
import (
7+
"errors"
8+
"io"
9+
"io/fs"
10+
"os"
11+
"path/filepath"
12+
"syscall"
13+
)
14+
15+
// HardLinkDir mirrors src into dst by hard-linking regular files when
16+
// possible, falling back to byte copying on cross-device setups (EXDEV).
17+
//
18+
// Hard links are O(1) — no data is transferred. They are safe for
19+
// mprcontents/ fixture trees because the Writer never modifies existing
20+
// .mxunit files in place; it only creates new files with fresh UUIDs
21+
// (and updateUnit now uses atomic rename, not direct WriteFile).
22+
//
23+
// Falls back transparently to CopyFile on cross-device setups so tests
24+
// work correctly regardless of where /tmp is mounted relative to testdata.
25+
func HardLinkDir(src, dst string) error {
26+
return filepath.WalkDir(src, func(p string, d fs.DirEntry, err error) error {
27+
if err != nil {
28+
return err
29+
}
30+
rel, err := filepath.Rel(src, p)
31+
if err != nil {
32+
return err
33+
}
34+
target := filepath.Join(dst, rel)
35+
if d.IsDir() {
36+
return os.MkdirAll(target, 0o755)
37+
}
38+
if linkErr := os.Link(p, target); linkErr != nil {
39+
if errors.Is(linkErr, syscall.EXDEV) {
40+
return CopyFile(p, target)
41+
}
42+
return linkErr
43+
}
44+
return nil
45+
})
46+
}
47+
48+
// CopyFile copies src to dst using io.Copy.
49+
func CopyFile(src, dst string) error {
50+
in, err := os.Open(src)
51+
if err != nil {
52+
return err
53+
}
54+
defer in.Close()
55+
if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
56+
return err
57+
}
58+
out, err := os.Create(dst)
59+
if err != nil {
60+
return err
61+
}
62+
defer out.Close()
63+
if _, err := io.Copy(out, in); err != nil {
64+
return err
65+
}
66+
return nil
67+
}

mdl/backend/mpr/factory_test.go

Lines changed: 4 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,12 @@
33
package mprbackend
44

55
import (
6-
"errors"
7-
"io"
8-
"io/fs"
96
"os"
107
"path/filepath"
118
"runtime"
12-
"syscall"
139
"testing"
1410

11+
"github.com/mendixlabs/mxcli/internal/testfsutil"
1512
mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr"
1613
)
1714

@@ -338,7 +335,7 @@ func TestNewExecutorContextWithReferences_BothServicesWired(t *testing.T) {
338335
}
339336
}
340337

341-
// --- helpers (mirror mdl/backend/mpr/repos/testhelpers_test.go) ---
338+
// --- fixture helpers ---
342339

343340
func copyFixture(t *testing.T, srcMPR, dstDir string) string {
344341
t.Helper()
@@ -351,76 +348,15 @@ func copyFixture(t *testing.T, srcMPR, dstDir string) string {
351348

352349
func copyMPRTree(srcMPR, dstDir string) (string, error) {
353350
dstMPR := filepath.Join(dstDir, filepath.Base(srcMPR))
354-
if err := copyOneFile(srcMPR, dstMPR); err != nil {
351+
if err := testfsutil.CopyFile(srcMPR, dstMPR); err != nil {
355352
return "", err
356353
}
357354
srcContents := filepath.Join(filepath.Dir(srcMPR), "mprcontents")
358355
if info, err := os.Stat(srcContents); err == nil && info.IsDir() {
359356
dstContents := filepath.Join(dstDir, "mprcontents")
360-
if err := hardLinkDir(srcContents, dstContents); err != nil {
357+
if err := testfsutil.HardLinkDir(srcContents, dstContents); err != nil {
361358
return "", err
362359
}
363360
}
364361
return dstMPR, nil
365362
}
366-
367-
// hardLinkDir mirrors src into dst using hard links; falls back to copy on EXDEV.
368-
func hardLinkDir(src, dst string) error {
369-
return filepath.WalkDir(src, func(p string, d fs.DirEntry, err error) error {
370-
if err != nil {
371-
return err
372-
}
373-
rel, err := filepath.Rel(src, p)
374-
if err != nil {
375-
return err
376-
}
377-
target := filepath.Join(dst, rel)
378-
if d.IsDir() {
379-
return os.MkdirAll(target, 0o755)
380-
}
381-
if linkErr := os.Link(p, target); linkErr != nil {
382-
if errors.Is(linkErr, syscall.EXDEV) {
383-
return copyOneFile(p, target)
384-
}
385-
return linkErr
386-
}
387-
return nil
388-
})
389-
}
390-
391-
func copyOneFile(src, dst string) error {
392-
in, err := os.Open(src)
393-
if err != nil {
394-
return err
395-
}
396-
defer in.Close()
397-
if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
398-
return err
399-
}
400-
out, err := os.Create(dst)
401-
if err != nil {
402-
return err
403-
}
404-
defer out.Close()
405-
if _, err := io.Copy(out, in); err != nil {
406-
return err
407-
}
408-
return nil
409-
}
410-
411-
func copyDir(src, dst string) error {
412-
return filepath.WalkDir(src, func(p string, d fs.DirEntry, err error) error {
413-
if err != nil {
414-
return err
415-
}
416-
rel, err := filepath.Rel(src, p)
417-
if err != nil {
418-
return err
419-
}
420-
target := filepath.Join(dst, rel)
421-
if d.IsDir() {
422-
return os.MkdirAll(target, 0o755)
423-
}
424-
return copyOneFile(p, target)
425-
})
426-
}

mdl/backend/mpr/repos/testhelpers_test.go

Lines changed: 9 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -3,20 +3,17 @@
33
package mprrepos
44

55
import (
6-
"errors"
7-
"io"
8-
"io/fs"
96
"os"
107
"path/filepath"
118
"runtime"
12-
"syscall"
139
"testing"
1410

11+
"github.com/mendixlabs/mxcli/internal/testfsutil"
1512
"github.com/mendixlabs/mxcli/model"
1613
mmpr "github.com/mendixlabs/mxcli/modelsdk/mpr"
1714
)
1815

19-
// fixtureOpenSem limits concurrent fixture open operations at GOMAXPROCS
16+
// fixtureOpenSem limits concurrent fixture open operations to GOMAXPROCS
2017
// to prevent I/O storms when many tests run in parallel.
2118
var fixtureOpenSem = make(chan struct{}, runtime.GOMAXPROCS(0))
2219

@@ -28,16 +25,12 @@ func typedID(s string) model.ID { return model.ID(s) }
2825
// relative to test files in mdl/backend/mpr/repos/.
2926
const fixturePath = "../../../../testdata/expr-checker/minimal.mpr"
3027

31-
// openTestWriter sets up an isolated writable copy of the fixture for
32-
// a test and opens it as a *mmpr.Writer.
28+
// openTestWriter sets up an isolated writable copy of the fixture and
29+
// opens it as a *mmpr.Writer.
3330
//
34-
// Isolation strategy for mprcontents/ (370 immutable .mxunit files):
35-
// - Hard-link instead of copy: O(1) per file, no data written
36-
// - The Writer never modifies existing .mxunit files; it only creates
37-
// new ones with fresh UUIDs, so hard links are safe
38-
//
39-
// The .mpr SQLite file (68 KB) must be a real copy because the Writer
40-
// commits WAL changes back into it.
31+
// The .mpr SQLite file is real-copied (Writer writes WAL back to it).
32+
// mprcontents/ is hard-linked via testfsutil.HardLinkDir — O(1) per
33+
// file with automatic EXDEV fallback to byte-copy on cross-device setups.
4134
func openTestWriter(t *testing.T) *mmpr.Writer {
4235
t.Helper()
4336
t.Parallel()
@@ -61,86 +54,17 @@ func copyFixture(t *testing.T, srcMPR, dstDir string) string {
6154
return dst
6255
}
6356

64-
// copyMPRTree copies the .mpr SQLite file and hard-links the mprcontents/
65-
// directory tree into dstDir. Hard links are used for mprcontents/ because
66-
// those files are immutable once written — the Writer adds new files rather
67-
// than modifying existing ones.
6857
func copyMPRTree(srcMPR, dstDir string) (string, error) {
6958
dstMPR := filepath.Join(dstDir, filepath.Base(srcMPR))
70-
if err := copyOneFile(srcMPR, dstMPR); err != nil {
59+
if err := testfsutil.CopyFile(srcMPR, dstMPR); err != nil {
7160
return "", err
7261
}
7362
srcContents := filepath.Join(filepath.Dir(srcMPR), "mprcontents")
7463
if info, err := os.Stat(srcContents); err == nil && info.IsDir() {
7564
dstContents := filepath.Join(dstDir, "mprcontents")
76-
if err := hardLinkDir(srcContents, dstContents); err != nil {
65+
if err := testfsutil.HardLinkDir(srcContents, dstContents); err != nil {
7766
return "", err
7867
}
7968
}
8069
return dstMPR, nil
8170
}
82-
83-
// hardLinkDir mirrors src into dst using hard links when possible.
84-
// Falls back to copyOneFile on cross-device setups (EXDEV). Hard links
85-
// are safe for mprcontents/ because the Writer only creates new files
86-
// with fresh UUIDs — it never modifies existing .mxunit files in place.
87-
func hardLinkDir(src, dst string) error {
88-
return filepath.WalkDir(src, func(p string, d fs.DirEntry, err error) error {
89-
if err != nil {
90-
return err
91-
}
92-
rel, err := filepath.Rel(src, p)
93-
if err != nil {
94-
return err
95-
}
96-
target := filepath.Join(dst, rel)
97-
if d.IsDir() {
98-
return os.MkdirAll(target, 0o755)
99-
}
100-
if linkErr := os.Link(p, target); linkErr != nil {
101-
if errors.Is(linkErr, syscall.EXDEV) {
102-
return copyOneFile(p, target)
103-
}
104-
return linkErr
105-
}
106-
return nil
107-
})
108-
}
109-
110-
func copyOneFile(src, dst string) error {
111-
in, err := os.Open(src)
112-
if err != nil {
113-
return err
114-
}
115-
defer in.Close()
116-
if err := os.MkdirAll(filepath.Dir(dst), 0o755); err != nil {
117-
return err
118-
}
119-
out, err := os.Create(dst)
120-
if err != nil {
121-
return err
122-
}
123-
defer out.Close()
124-
if _, err := io.Copy(out, in); err != nil {
125-
return err
126-
}
127-
return nil
128-
}
129-
130-
// copyDir is retained for tests that explicitly need a real on-disk copy.
131-
func copyDir(src, dst string) error {
132-
return filepath.WalkDir(src, func(p string, d fs.DirEntry, err error) error {
133-
if err != nil {
134-
return err
135-
}
136-
rel, err := filepath.Rel(src, p)
137-
if err != nil {
138-
return err
139-
}
140-
target := filepath.Join(dst, rel)
141-
if d.IsDir() {
142-
return os.MkdirAll(target, 0o755)
143-
}
144-
return copyOneFile(p, target)
145-
})
146-
}

0 commit comments

Comments
 (0)