Skip to content

Commit ffab4bb

Browse files
mlwellesShiva
andauthored
fix(test): resolve macOS binary selection and test data clobbering (#9606)
**Description** This PR builds upon [#9603](#9603) by @shiva-istari and adds fixes for issues exposed when macOS skip guards are removed. ### Original PR (#9603) > On macOS, setupBinary() now copies both the Linux binary (for Docker containers) and the host-native binary (for local bulk/live loader commands) into tempBinDir. On Linux, a single binary serves both purposes. BulkLoad() and LiveLoad() use hostDgraphBinaryPath() to pick the correct one based on the platform. Removed macOS skip guards from 13 test files so all tests run on both platforms after make install. ### Fix 1: test data clobbering in `--suite=all` (`t/t.go`) When running `make test` (`--suite=all`), the `load` and `ldbc` download blocks shared `*tmp`. The LDBC block's `MakeDirEmpty` wiped load data files, causing `systest/1million` to fail. Fixed by hoisting directory init above both download blocks so `MakeDirEmpty` runs once. Also uses a dedicated subdirectory (`dgraph-test-data`) instead of bare `os.TempDir()`. ### Fix 2: `$GOPATH/bin` in Docker Compose files (30 files) 30 Docker Compose files hardcoded `$GOPATH/bin` as the binary mount source. On macOS this mounts the native binary into Linux containers, crashing them. Replaced all 78 occurrences with `${LINUX_GOBIN:-$GOPATH/bin}` to match the pattern in `dgraph/docker-compose.yml`. ### Fix 3: add `--timeout` flag to t/ runner The per-package test timeout was hardcoded to 30m, which is too short for the `21million/live` test on macOS (where Docker runs in a VM with I/O overhead). Added a `--timeout` flag to `t/t.go` and wired it through the Makefile as `TIMEOUT`: ```bash make test TIMEOUT=90m cd t && ./t --suite=all --timeout=60m ``` Defaults remain unchanged: 30m normal, 180m with `--race`. An explicit `--timeout` overrides both. Updated docs in Makefile help, CONTRIBUTING.md, and TESTING.md. **Checklist** - [x] The PR title follows the [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/#summary) syntax - [x] Code compiles correctly and linting passes locally --------- Co-authored-by: Shiva <[email protected]>
1 parent 9ffab52 commit ffab4bb

50 files changed

Lines changed: 190 additions & 214 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

CONTRIBUTING.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,7 @@ make test-upgrade # Upgrade tests
154154

155155
# Use variables for more control
156156
make test TAGS=integration2 PKG=systest/vector
157+
make test TIMEOUT=90m # Override per-package timeout (default: 30m)
157158
```
158159

159160
Run `make help` to see all available targets and variables.

Makefile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ else
9898
endif
9999
else
100100
@echo "Running test suite: $(or $(SUITE),all)"
101-
$(MAKE) -C t test args="--suite=$(or $(SUITE),all) $(if $(PKG),--pkg=\"$(PKG)\") $(if $(TEST),--test=\"$(TEST)\")"
101+
$(MAKE) -C t test args="--suite=$(or $(SUITE),all) $(if $(PKG),--pkg=\"$(PKG)\") $(if $(TEST),--test=\"$(TEST)\") $(if $(TIMEOUT),--timeout=$(TIMEOUT))"
102102
endif
103103

104104
.PHONY: test-all
@@ -205,6 +205,7 @@ help: ## Show available targets and variables
205205
@echo " TAGS Go build tags - bypasses t/ runner (e.g., make test TAGS=integration2)"
206206
@echo " PKG Limit to specific package (e.g., make test PKG=systest/export)"
207207
@echo " TEST Run specific test function (e.g., make test TEST=TestGQLSchema)"
208+
@echo " TIMEOUT Per-package test timeout (e.g., make test TIMEOUT=60m). Default: 30m"
208209
@echo " FUZZ Enable fuzz testing (e.g., make test FUZZ=1)"
209210
@echo " FUZZTIME Fuzz duration per package (e.g., make test FUZZ=1 FUZZTIME=60s)"
210211
@echo ""
@@ -223,4 +224,5 @@ help: ## Show available targets and variables
223224
@echo " make test TAGS=upgrade PKG=acl TEST=TestACL # specific upgrade test"
224225
@echo " make test FUZZ=1 PKG=dql FUZZTIME=30s # fuzz dql package for 30s"
225226
@echo " make test SUITE=systest PKG=systest/backup/filesystem # systest for backup pkg"
227+
@echo " make test TIMEOUT=90m # all suites with 90m timeout"
226228
@echo " make test-benchmark PKG=posting # benchmark posting package"

TESTING.md

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,7 @@ For more control, pass variables to `make test`:
335335
| `TAGS` | Go build tags - bypasses t/ runner | `make test TAGS=integration2` |
336336
| `PKG` | Limit to specific package | `make test PKG=systest/export` |
337337
| `TEST` | Run specific test function | `make test TEST=TestGQLSchema` |
338+
| `TIMEOUT` | Per-package test timeout | `make test TIMEOUT=90m` |
338339
| `FUZZ` | Enable fuzz testing | `make test FUZZ=1` |
339340
| `FUZZTIME` | Fuzz duration per package | `make test FUZZ=1 FUZZTIME=60s` |
340341

@@ -578,15 +579,16 @@ cd t && go build .
578579

579580
### Key Flags
580581

581-
| Flag | Description |
582-
| ------------- | ------------------------------------------------------------------ |
583-
| `--suite=X` | Select test suite(s): all, ldbc, load, unit, systest, vector, core |
584-
| `--pkg=X` | Run specific package |
585-
| `--test=X` | Run specific test function |
586-
| `-j=N` | Concurrency (default: 1) |
587-
| `--keep` | Keep cluster running after tests |
588-
| `-r` | Remove all test containers |
589-
| `--skip-slow` | Skip slow packages |
582+
| Flag | Description |
583+
| ------------- | ------------------------------------------------------------------- |
584+
| `--suite=X` | Select test suite(s): all, ldbc, load, unit, systest, vector, core |
585+
| `--pkg=X` | Run specific package |
586+
| `--test=X` | Run specific test function |
587+
| `--timeout=X` | Per-package timeout (e.g. 60m, 2h). Default: 30m (180m with --race) |
588+
| `-j=N` | Concurrency (default: 1) |
589+
| `--keep` | Keep cluster running after tests |
590+
| `-r` | Remove all test containers |
591+
| `--skip-slow` | Skip slow packages |
590592

591593
---
592594

check_upgrade/check_upgrade_test.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,8 @@ package checkupgrade
1010
import (
1111
"context"
1212
"fmt"
13-
"os"
1413
"os/exec"
15-
"path/filepath"
1614
"regexp"
17-
"runtime"
1815
"testing"
1916
"time"
2017

@@ -28,11 +25,6 @@ import (
2825
)
2926

3027
func TestCheckUpgrade(t *testing.T) {
31-
if runtime.GOOS != "linux" && os.Getenv("DGRAPH_BINARY") == "" {
32-
fmt.Println("Skipping live load-uids tests on non-Linux platforms due to dgraph binary dependency")
33-
fmt.Println("You can set the DGRAPH_BINARY environment variable to path of a native dgraph binary to run these tests")
34-
os.Exit(0)
35-
}
3628
conf := dgraphtest.NewClusterConfig().WithNumAlphas(1).WithNumZeros(1).WithReplicas(1).
3729
WithACL(time.Hour).WithVersion("57aa5c4ac")
3830
c, err := dgraphtest.NewLocalCluster(conf)
@@ -88,7 +80,7 @@ func TestCheckUpgrade(t *testing.T) {
8880
"--namespace", "1",
8981
}
9082

91-
cmd := exec.Command(filepath.Join(c1.GetTempDir(), "dgraph"), args...)
83+
cmd := exec.Command(c1.HostDgraphBinaryPath(), args...)
9284
out, err := cmd.CombinedOutput()
9385
require.NoError(t, err)
9486
actualOutput := string(out)
@@ -108,7 +100,7 @@ func TestQueryDuplicateNodes(t *testing.T) {
108100
WithACL(time.Hour).WithVersion("57aa5c4ac").WithAclAlg(jwt.GetSigningMethod("HS256"))
109101
c, err := dgraphtest.NewLocalCluster(conf)
110102
require.NoError(t, err)
111-
// defer func() { c.Cleanup(t.Failed()) }()
103+
defer func() { c.Cleanup(t.Failed()) }()
112104
require.NoError(t, c.Start())
113105
gc, cleanup, err := c.Client()
114106
require.NoError(t, err)

dgraph/cmd/alpha/mutations_mode/docker-compose.yml

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ services:
1313
- "9080"
1414
volumes:
1515
- type: bind
16-
source: $GOPATH/bin
16+
source: ${LINUX_GOBIN:-$GOPATH/bin}
1717
target: /gobin
1818
read_only: true
1919
command:
@@ -30,7 +30,7 @@ services:
3030
- "9080"
3131
volumes:
3232
- type: bind
33-
source: $GOPATH/bin
33+
source: ${LINUX_GOBIN:-$GOPATH/bin}
3434
target: /gobin
3535
read_only: true
3636
command:
@@ -47,7 +47,7 @@ services:
4747
- "9080"
4848
volumes:
4949
- type: bind
50-
source: $GOPATH/bin
50+
source: ${LINUX_GOBIN:-$GOPATH/bin}
5151
target: /gobin
5252
read_only: true
5353
command:
@@ -64,7 +64,7 @@ services:
6464
- "6080"
6565
volumes:
6666
- type: bind
67-
source: $GOPATH/bin
67+
source: ${LINUX_GOBIN:-$GOPATH/bin}
6868
target: /gobin
6969
read_only: true
7070
command:
@@ -82,7 +82,7 @@ services:
8282
- "6080"
8383
volumes:
8484
- type: bind
85-
source: $GOPATH/bin
85+
source: ${LINUX_GOBIN:-$GOPATH/bin}
8686
target: /gobin
8787
read_only: true
8888
command:
@@ -100,7 +100,7 @@ services:
100100
- "6080"
101101
volumes:
102102
- type: bind
103-
source: $GOPATH/bin
103+
source: ${LINUX_GOBIN:-$GOPATH/bin}
104104
target: /gobin
105105
read_only: true
106106
command:

dgraph/cmd/dgraphimport/import_test.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,7 @@ import (
1010
"context"
1111
"encoding/json"
1212
"fmt"
13-
"os"
1413
"path/filepath"
15-
"runtime"
1614
"strconv"
1715
"strings"
1816
"testing"
@@ -326,11 +324,6 @@ func runImportTest(t *testing.T, tt testcase) {
326324

327325
// setupBulkCluster creates and configures a cluster for bulk loading data
328326
func setupBulkCluster(t *testing.T, numAlphas int, encrypted bool) (*dgraphtest.LocalCluster, string) {
329-
if runtime.GOOS != "linux" && os.Getenv("DGRAPH_BINARY") == "" {
330-
fmt.Println("You can set the DGRAPH_BINARY environment variable to path of a native dgraph binary to run these tests")
331-
t.Skip("Skipping test on non-Linux platforms due to dgraph binary dependency")
332-
}
333-
334327
baseDir := t.TempDir()
335328
bulkConf := dgraphtest.NewClusterConfig().
336329
WithNumAlphas(numAlphas).

dgraph/cmd/live/load-json/load_test.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ package live
99

1010
import (
1111
"context"
12-
"fmt"
1312
"os"
1413
"path/filepath"
1514
"runtime"
@@ -162,12 +161,6 @@ func TestLiveLoadJSONMultipleFiles(t *testing.T) {
162161
}
163162

164163
func TestMain(m *testing.M) {
165-
if runtime.GOOS != "linux" && os.Getenv("DGRAPH_BINARY") == "" {
166-
fmt.Println("Skipping live load-json tests on non-Linux platforms due to dgraph binary dependency")
167-
fmt.Println("You can set the DGRAPH_BINARY environment variable to path of a native dgraph binary to run these tests")
168-
os.Exit(0)
169-
}
170-
171164
_, thisFile, _, _ := runtime.Caller(0)
172165
testDataDir = filepath.Dir(thisFile)
173166

dgraph/cmd/live/load-uids/load_test.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -351,12 +351,6 @@ func TestLiveLoadFileNameMultipleCorrect(t *testing.T) {
351351
}
352352

353353
func TestMain(m *testing.M) {
354-
if runtime.GOOS != "linux" && os.Getenv("DGRAPH_BINARY") == "" {
355-
fmt.Println("Skipping live load-uids tests on non-Linux platforms due to dgraph binary dependency")
356-
fmt.Println("You can set the DGRAPH_BINARY environment variable to path of a native dgraph binary to run these tests")
357-
os.Exit(0)
358-
}
359-
360354
alphaService = testutil.GetSockAddr()
361355
alphaName = testutil.Instance
362356

dgraph/cmd/version/version_test.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@
66
package version
77

88
import (
9-
"fmt"
109
"os"
1110
"path/filepath"
12-
"runtime"
1311
"testing"
1412

1513
"github.com/stretchr/testify/require"
@@ -19,11 +17,6 @@ import (
1917

2018
// Test `dgraph version` with an empty config file.
2119
func TestMain(m *testing.M) {
22-
if runtime.GOOS != "linux" && os.Getenv("DGRAPH_BINARY") == "" {
23-
fmt.Println("Skipping version tests on non-Linux platforms due to dgraph binary dependency")
24-
fmt.Println("You can set the DGRAPH_BINARY environment variable to path of a native dgraph binary to run these tests")
25-
os.Exit(0)
26-
}
2720
m.Run()
2821
}
2922

dgraphtest/image.go

Lines changed: 39 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"os"
1313
"os/exec"
1414
"path/filepath"
15+
"runtime"
1516
"strings"
1617
"sync"
1718

@@ -28,9 +29,18 @@ func (c *LocalCluster) dgraphImage() string {
2829
return "dgraph/dgraph:local"
2930
}
3031

31-
// setupBinary sets up the dgraph binary. The binary is expected to be a version
32-
// compiled that is compatible with the host OS and architecture. Search this repo
33-
// for DGRAPH_BINARY to learn its use.
32+
// setupBinary sets up dgraph binaries in tempBinDir.
33+
//
34+
// On Linux a single "dgraph" binary from $GOPATH/bin serves both Docker
35+
// containers and local commands (bulk/live loader).
36+
//
37+
// On non-Linux (macOS) two binaries are placed in tempBinDir:
38+
// - "dgraph" – a Linux binary for Docker containers, from
39+
// $GOPATH/linux_<arch> (or LINUX_GOBIN if set).
40+
// - "dgraph_host" – the host-native binary for local commands,
41+
// from $GOPATH/bin.
42+
//
43+
// Both are produced by "make install".
3444
func (c *LocalCluster) setupBinary() error {
3545
if err := ensureDgraphClone(); err != nil {
3646
panic(err)
@@ -43,11 +53,34 @@ func (c *LocalCluster) setupBinary() error {
4353
}
4454
}
4555
if c.conf.version == localVersion {
46-
if os.Getenv("GOPATH") == "" {
56+
gopath := os.Getenv("GOPATH")
57+
if gopath == "" {
4758
return errors.New("GOPATH is not set")
4859
}
49-
fromDir := filepath.Join(os.Getenv("GOPATH"), "bin")
50-
return copyBinary(fromDir, c.tempBinDir, c.conf.version)
60+
61+
if runtime.GOOS == "linux" {
62+
// On Linux $GOPATH/bin/dgraph is both the native and Docker binary.
63+
return copyBinary(filepath.Join(gopath, "bin"), c.tempBinDir, c.conf.version)
64+
}
65+
66+
// Non-Linux (macOS): need separate Linux and host-native binaries.
67+
// 1. Copy the Linux binary (for Docker containers) as "dgraph".
68+
linuxDir := os.Getenv("LINUX_GOBIN")
69+
if linuxDir == "" {
70+
linuxDir = filepath.Join(gopath, "linux_"+runtime.GOARCH)
71+
}
72+
if err := copyBinary(linuxDir, c.tempBinDir, c.conf.version); err != nil {
73+
return err
74+
}
75+
76+
// 2. Copy the host-native binary (for local bulk/live commands) as "dgraph_host".
77+
hostSrc := filepath.Join(gopath, "bin", "dgraph")
78+
79+
hostDst := filepath.Join(c.tempBinDir, "dgraph_host")
80+
if err := copy(hostSrc, hostDst); err != nil {
81+
return errors.Wrapf(err, "error copying host-native binary from [%v] to [%v]", hostSrc, hostDst)
82+
}
83+
return nil
5184
}
5285

5386
binaryPath := filepath.Join(binariesPath, fmt.Sprintf(binaryNameFmt, c.conf.version))

0 commit comments

Comments
 (0)