fix(builder): write the eest build sidecar even when the fill fails

pkg/builder/eest_payloads.go

@@ -255,18 +255,26 @@ func (b *EESTPayloadsBuilder) Build(ctx context.Context, name string, opts Build
 		return false, err
 	}
 
-	if err := b.run(ctx, log, target); err != nil {
-		return false, err
-	}
+	runErr := b.run(ctx, log, target)
 
 	// Record the config fingerprint (computed pre-run) for a later
-	// --rebuild-on-diff run. Best-effort; a failure must not fail the build.
+	// --rebuild-on-diff run. Written even when the fill failed: fill-stateful
+	// continues through individual test failures and still writes every fixture
+	// it did produce, so a target where 1 of 100 tests failed to fill must still
+	// be described on disk (the build summary reads this sidecar for the EEST
+	// repo, tests and marker). An output_dir holding only sidecars still counts
+	// as unpopulated, so this can't make a later build skip a target that
+	// produced nothing. Best-effort; a failure here must not mask runErr.
 	if inputsErr != nil {
 		log.WithError(inputsErr).Warn("Failed to compute build fingerprint; sidecar not written")
 	} else if err := writeBuildSidecar(target.OutputDir, EESTPayloadsBuilderName, inputs); err != nil {
 		log.WithError(err).Warn("Failed to write build fingerprint sidecar")
 	}
 
+	if runErr != nil {
+		return false, runErr
+	}
+
 	return false, nil
 }
 
@@ -828,12 +836,12 @@ func (b *EESTPayloadsBuilder) buildFillImage(ctx context.Context, log logrus.Fie
 const (
 	// eestFillResultFile is the sidecar written next to the fixtures recording
 	// how many tests the fill produced/failed (read by the build markdown summary).
-	eestFillResultFile = ".benchmarkoor-fill.json"
+	eestFillResultFile = sidecarPrefix + "fill.json"
 
 	// pytestReportFile is the pytest-json-report output written under output_dir
 	// (via PYTEST_ADDOPTS --json-report); it carries the authoritative
 	// passed/failed tally. Relative to output_dir (= the fill container's /out).
-	pytestReportFile = ".benchmarkoor-pytest-report.json"
+	pytestReportFile = sidecarPrefix + "pytest-report.json"
 )
 
 // dirSize returns the total size in bytes of all regular files under dir.

pkg/builder/eest_payloads_test.go

@@ -376,6 +376,38 @@ func TestEESTPayloadsBuilder_BuildSkipsPopulatedDir(t *testing.T) {
 	assert.FileExists(t, filepath.Join(dir, eestFillResultFile))
 }
 
+// A build that failed before producing any fixture still leaves its sidecars
+// behind (the fingerprint is now written even when the fill errors). Those must
+// not make the next build skip the target as "already populated".
+func TestEESTPayloadsBuilder_BuildDoesNotSkipSidecarOnlyDir(t *testing.T) {
+	t.Setenv("SCHELK_STATE", filepath.Join(t.TempDir(), "absent.json"))
+
+	dir := t.TempDir()
+	for _, name := range []string{buildSidecarFile, eestFillResultFile} {
+		require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte("{}"), 0o600))
+	}
+
+	cfg := &config.EESTPayloadsConfig{
+		FillImage: "fill:latest",
+		// A local, non-existent repo path keeps the lazy EEST SHA resolution
+		// offline: `git ls-remote` fails immediately instead of hitting the network.
+		EESTRepo: filepath.Join(t.TempDir(), "no-such-repo"),
+		Targets: []config.EESTPayloadTarget{{
+			Name: "compute", FillerClient: "geth", SourceDir: filepath.Join(t.TempDir(), "missing"),
+			OutputDir: dir, Fork: "Osaka", FillerImage: "geth:master",
+			Tests: []string{"tests/benchmark/compute"},
+		}},
+	}
+
+	b := NewEESTPayloadsBuilder(noopLogger(), cfg, "docker", &fakeMgr{}, t.TempDir())
+
+	// Not skipped: it proceeds into the build and fails on the absent source_dir.
+	skipped, err := b.Build(context.Background(), "compute", BuildOptions{})
+	require.Error(t, err)
+	assert.False(t, skipped, "a sidecar-only output_dir must not count as built")
+	assert.Contains(t, err.Error(), "source_dir")
+}
+
 func TestMaterializeAddressStubs(t *testing.T) {
 	t.Setenv("TMPDIR", t.TempDir())
 

pkg/builder/fingerprint.go

@@ -15,7 +15,7 @@ import (
 // at an output_dir root after a successful build. When --rebuild-on-diff is set,
 // the next build compares the current config fingerprint against this file to
 // decide whether the output is stale and must be rebuilt.
-const buildSidecarFile = ".benchmarkoor-build.json"
+const buildSidecarFile = sidecarPrefix + "build.json"
 
 // buildFingerprintSchema versions the sidecar format so a future change can
 // invalidate old sidecars deliberately.

pkg/builder/util.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"strings"
 	"sync"
 	"time"
 
@@ -28,8 +29,18 @@ func mountTempDir() string {
 	return os.TempDir()
 }
 
-// isPopulated reports whether dir exists and contains at least one
-// entry. A missing dir returns (false, nil).
+// sidecarPrefix marks the bookkeeping files benchmarkoor itself writes into an
+// output_dir (the build fingerprint, the eest fill result, the pytest report).
+// They describe a build rather than being its product.
+const sidecarPrefix = ".benchmarkoor-"
+
+// isPopulated reports whether dir exists and holds at least one entry a builder
+// actually produced. A missing dir returns (false, nil).
+//
+// benchmarkoor's own sidecars don't count: they are written even when a build
+// fails before producing anything, so a dir holding nothing else must still read
+// as empty — otherwise the next build would skip it as "already populated" and
+// the target would never be produced.
 func isPopulated(dir string) (bool, error) {
 	entries, err := os.ReadDir(dir)
 	if err != nil {
@@ -40,7 +51,13 @@ func isPopulated(dir string) (bool, error) {
 		return false, fmt.Errorf("reading output_dir %q: %w", dir, err)
 	}
 
-	return len(entries) > 0, nil
+	for _, entry := range entries {
+		if !strings.HasPrefix(entry.Name(), sidecarPrefix) {
+			return true, nil
+		}
+	}
+
+	return false, nil
 }
 
 // prepareOutputDir ensures dir exists. When force is true the directory is

pkg/builder/util_test.go

@@ -1,12 +1,52 @@
 package builder
 
 import (
+	"os"
+	"path/filepath"
 	"sync"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
+func TestIsPopulated(t *testing.T) {
+	t.Run("missing dir is not populated", func(t *testing.T) {
+		got, err := isPopulated(filepath.Join(t.TempDir(), "absent"))
+		require.NoError(t, err)
+		assert.False(t, got)
+	})
+
+	t.Run("empty dir is not populated", func(t *testing.T) {
+		got, err := isPopulated(t.TempDir())
+		require.NoError(t, err)
+		assert.False(t, got)
+	})
+
+	// A build that failed before producing anything still writes its sidecars.
+	// They must not make the dir look built, or the next build would skip it.
+	t.Run("sidecars alone are not populated", func(t *testing.T) {
+		dir := t.TempDir()
+		for _, name := range []string{buildSidecarFile, eestFillResultFile, pytestReportFile} {
+			require.NoError(t, os.WriteFile(filepath.Join(dir, name), []byte("{}"), 0o600))
+		}
+
+		got, err := isPopulated(dir)
+		require.NoError(t, err)
+		assert.False(t, got)
+	})
+
+	t.Run("any produced entry alongside sidecars is populated", func(t *testing.T) {
+		dir := t.TempDir()
+		require.NoError(t, os.WriteFile(filepath.Join(dir, buildSidecarFile), []byte("{}"), 0o600))
+		require.NoError(t, os.MkdirAll(filepath.Join(dir, ".meta"), 0o755))
+
+		got, err := isPopulated(dir)
+		require.NoError(t, err)
+		assert.True(t, got)
+	})
+}
+
 // TestTailBufferConcurrent exercises the case the mutex guards: the container
 // log-streaming goroutine keeps Write()-ing while the caller reads String() on
 // the error path (RunInitContainer doesn't join the streaming goroutine). Run