ethereum-optimism / ethereum-optimism/optimism

op-deployer forge package: tests can't pass in any single environment (PATH-dependent)

Open
#21,200 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
6.5k
Forks
4k
Avg merge
2d 18h
Merged PRs (30d)
134

Description

While auditing which Go packages are missing from the `go-tests` CI list (`TEST_PKGS`), I found `op-deployer/pkg/deployer/forge` has a test suite (7 test funcs) that has **never run in CI** and cannot pass in any single environment.

### Root cause

`StandardBin.Ensure` prefers a `forge` already on `PATH` when its version matches the pinned `StandardVersion`, before ever attempting a download:

```go
// op-deployer/pkg/deployer/forge/binary.go:193
// 2) PATH: use if version matches the pinned Version
if forgePath, err := exec.LookPath("forge"); err == nil {
if ver, err := getForgeVersion(ctx, forgePath); err == nil && ver == StandardVersion {
b.path = forgePath
return nil
}
}
```

This makes the package's tests mutually exclusive:

- **`forge` on PATH** (the CI/mise environment, which pins forge 1.2.3): `TestStandardBinary_Downloads` fails — `Ensure` short-circuits to the on-PATH binary instead of downloading.
- `download_OK`: `bin.Path()` returns the mise forge path, not `cacheDir/forge`.
- `invalid_checksum`: expects a `checksum mismatch` error but gets `nil` (download/checksum is never reached).
- **`forge` absent from PATH**: `TestMinimalSources`, `TestClient_Smoke`, `TestClient_OutputRedirection`, `TestScriptCaller` all fail with `could not find binary: exec: "forge": executable file not found in $PATH`.

### Repro

```
# With forge on PATH (e.g. via mise):
go test -tags=ci ./op-deployer/pkg/deployer/forge/... # TestStandardBinary_Downloads fails

# Without forge on PATH:
go test -tags=ci ./op-deployer/pkg/deployer/forge/... # smoke/MinimalSources fail
```

### Suggested fix

Isolate the download-path tests from the host `forge` — e.g. inject the `LookPath` lookup (or set `t.Setenv("PATH", "")`) in `TestStandardBinary_Downloads` so it exercises the download + checksum logic deterministically regardless of whether a matching forge is installed.

Once the package passes reliably, add `./op-deployer/pkg/deployer/forge/...` to `TEST_PKGS`.

Context: follow-up to #21197, which adds the gap packages that *do* pass.

---
*Filed by Claude (AI assistant) on Adrian's behalf.*

Contributor guide

Open the contributing guide

Research direction

Read op-deployer/pkg/deployer/forge/binary.go, especially StandardBin.Ensure, then run the package tests with and without forge on PATH. Focus on TestStandardBinary_Downloads and the smoke tests to make their environment assumptions deterministic. Done means the forge package passes reliably in both environments and ./op-deployer/pkg/deployer/forge/... is added to TEST_PKGS.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
ci-cd, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Clearly specified
Newbie friendliness
72/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.