Make the reconciler's recorded-state branches reachable from a test

These branches only run once a previous deploy recorded a fingerprint, and getEnvValue answers nothing without an azd client, so none of them had ever executed under test -- 38 call sites depend on it. The agents extension already solves this by serving the environment over gRPC the way azd does, so this borrows that: a small in-memory EnvironmentServiceServer, a real grpc.Server on an ephemeral port, and the actual client pointed at it.

That covers the pinned-version check three ways: a version the service no longer has is refused, one that is still there is reused unchanged, and a read that fails for any other reason leaves the pin alone rather than breaking a deploy.

The harness found a divergence on its first run: this extension's version-not-found hint still pointed at dataset list, which lists datasets rather than versions. The dataset extension's copy was corrected earlier today and this one was missed -- the tenth divergence between the two.

Author: m7md7sien

cli/azd/extensions/azure.ai.evaluations/internal/cmd/reconciler_env_test.go

@@ -0,0 +1,170 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+package cmd
+
+import (
+	"context"
+	"encoding/json"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"azureaieval/internal/pkg/dataset_api"
+	"azureaieval/internal/project"
+
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/policy"
+	"github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime"
+	"github.com/azure/azure-dev/cli/azd/pkg/azdext"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"google.golang.org/grpc"
+)
+
+// testEnvServer is an azd environment held in memory, so reconciliation paths
+// that only run once something was recorded at the last deploy are reachable
+// from a test. Without it, getEnvValue answers "" for everything and those
+// branches never execute.
+type testEnvServer struct {
+	azdext.UnimplementedEnvironmentServiceServer
+	values map[string]string
+}
+
+func (s *testEnvServer) GetValue(
+	_ context.Context, req *azdext.GetEnvRequest,
+) (*azdext.KeyValueResponse, error) {
+	return &azdext.KeyValueResponse{Value: s.values[req.Key]}, nil
+}
+
+func (s *testEnvServer) SetValue(
+	_ context.Context, req *azdext.SetEnvRequest,
+) (*azdext.EmptyResponse, error) {
+	if s.values == nil {
+		s.values = map[string]string{}
+	}
+	s.values[req.Key] = req.Value
+	return &azdext.EmptyResponse{}, nil
+}
+
+// newTestAzdClient serves the environment over gRPC the way azd itself does,
+// rather than faking the accessor, so the client code under test is the real one.
+func newTestAzdClient(t *testing.T, env *testEnvServer) *azdext.AzdClient {
+	t.Helper()
+
+	server := grpc.NewServer()
+	azdext.RegisterEnvironmentServiceServer(server, env)
+
+	listener, err := net.Listen("tcp", "127.0.0.1:0")
+	require.NoError(t, err)
+	go func() { _ = server.Serve(listener) }()
+	t.Cleanup(func() {
+		server.Stop()
+		_ = listener.Close()
+	})
+
+	client, err := azdext.NewAzdClient(azdext.WithAddress(listener.Addr().String()))
+	require.NoError(t, err)
+	t.Cleanup(func() { client.Close() })
+
+	return client
+}
+
+// pinnedDatasetReconciler builds a reconciler whose environment already holds
+// what a previous deploy recorded for a dataset, and whose service answers a
+// version read with the given status.
+func pinnedDatasetReconciler(
+	t *testing.T, name, version string, versionStatus int,
+) (*evalReconciler, string) {
+	t.Helper()
+
+	dir := t.TempDir()
+	localPath := filepath.Join(dir, name+".jsonl")
+	require.NoError(t, os.WriteFile(localPath, []byte("{\"query\":\"hi\"}\n"), 0o600))
+
+	digest, err := project.Fingerprint(localPath)
+	require.NoError(t, err)
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if !strings.Contains(r.URL.Path, "/versions/") {
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+		if versionStatus != http.StatusOK {
+			w.WriteHeader(versionStatus)
+			return
+		}
+		w.Header().Set("Content-Type", "application/json")
+		require.NoError(t, json.NewEncoder(w).Encode(map[string]any{
+			"name": name, "version": version,
+		}))
+	}))
+	t.Cleanup(srv.Close)
+
+	env := &testEnvServer{values: map[string]string{
+		project.FingerprintKey("dataset", name): digest,
+		versionKey("dataset", name):             version,
+	}}
+
+	pipeline := runtime.NewPipeline("test", "v1", runtime.PipelineOptions{},
+		&policy.ClientOptions{Retry: policy.RetryOptions{MaxRetries: -1}})
+
+	return &evalReconciler{ec: &evalContext{
+		azdClient:     newTestAzdClient(t, env),
+		envName:       "test",
+		datasetClient: dataset_api.NewDatasetClientFromPipeline(srv.URL, pipeline),
+	}}, localPath
+}
+
+// A pin settles which version to use, not whether it is still there. Reusing it
+// unread let a deleted version report as unchanged while the eval pointed at
+// nothing, which is what `create` did straight after `dataset delete`.
+func TestEnsureDatasetRefusesAPinnedVersionTheServiceNoLongerHas(t *testing.T) {
+	r, localPath := pinnedDatasetReconciler(t, "golden", "1.0", http.StatusNotFound)
+
+	_, _, err := r.EnsureDataset(
+		context.Background(),
+		project.DatasetDecl{Name: "golden", Version: "1.0"},
+		localPath,
+	)
+
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "1.0", "the version that is missing")
+	assert.Contains(t, err.Error(), "versions list", "and the command that shows what is there")
+}
+
+// The ordinary case: the pin is still registered, so reconciliation reuses it
+// and reports no change.
+func TestEnsureDatasetReusesAPinnedVersionThatStillExists(t *testing.T) {
+	r, localPath := pinnedDatasetReconciler(t, "golden", "1.0", http.StatusOK)
+
+	version, changed, err := r.EnsureDataset(
+		context.Background(),
+		project.DatasetDecl{Name: "golden", Version: "1.0"},
+		localPath,
+	)
+
+	require.NoError(t, err)
+	assert.Equal(t, "1.0", version)
+	assert.False(t, changed, "an unchanged file at a pinned version publishes nothing")
+}
+
+// A read that failed is not a read that came back empty. Failing the deploy on
+// a 403 or a timeout would turn a transient service problem into a broken
+// pipeline for a pin that is very probably fine.
+func TestEnsureDatasetKeepsAPinnedVersionWhenTheReadFails(t *testing.T) {
+	r, localPath := pinnedDatasetReconciler(t, "golden", "1.0", http.StatusForbidden)
+
+	version, changed, err := r.EnsureDataset(
+		context.Background(),
+		project.DatasetDecl{Name: "golden", Version: "1.0"},
+		localPath,
+	)
+
+	require.NoError(t, err)
+	assert.Equal(t, "1.0", version)
+	assert.False(t, changed)
+}

cli/azd/extensions/azure.ai.evaluations/internal/cmd/run_output.go

@@ -564,7 +564,9 @@ func writeResultsCSV(w io.Writer, run *eval_api.OpenAIEvalRun) error {
 	cw := csv.NewWriter(w)
 	defer cw.Flush()
 
-	if err := cw.Write([]string{"run_id", "status", "criterion", "passed", "failed"}); err != nil {
+	// Named as the service names it, and as the jsonl export already did, so a
+	// pipeline reading both formats needs one spelling rather than two.
+	if err := cw.Write([]string{"run_id", "status", "testing_criteria", "passed", "failed"}); err != nil {
 		return err
 	}
 	if len(run.PerTestingCriteria) == 0 {

cli/azd/extensions/azure.ai.evaluations/internal/cmd/run_output_write_test.go

@@ -38,7 +38,7 @@ func TestWriteResultsCSV(t *testing.T) {
 	rows, err := csv.NewReader(&buf).ReadAll()
 	require.NoError(t, err)
 
-	assert.Equal(t, []string{"run_id", "status", "criterion", "passed", "failed"}, rows[0])
+	assert.Equal(t, []string{"run_id", "status", "testing_criteria", "passed", "failed"}, rows[0])
 	assert.Equal(t, []string{"evalrun_abc", "completed", "task_adherence", "8", "2"}, rows[1])
 	assert.Equal(t, []string{"evalrun_abc", "completed", "coherence", "10", "0"}, rows[2])
 	assert.Len(t, rows, 3, "one header and one row per criterion")
@@ -57,7 +57,7 @@ func TestWriteResultsCSV_RunWithNoCriteria(t *testing.T) {
 	require.NoError(t, err)
 
 	require.Len(t, rows, 2)
-	assert.Equal(t, []string{"run_id", "status", "criterion", "passed", "failed"}, rows[0])
+	assert.Equal(t, []string{"run_id", "status", "testing_criteria", "passed", "failed"}, rows[0])
 	assert.Equal(t, []string{"evalrun_empty", "failed", "", "", ""}, rows[1])
 }
 

cli/azd/extensions/azure.ai.evaluations/internal/messages/messages.go

@@ -958,7 +958,7 @@ func DatasetNotFound(dataset string) error {
 func DatasetVersionNotFoundWithHint(dataset, version string) error {
 	return fmt.Errorf(
 		"no dataset %q at version %q in this project; "+
-			"`azd ai eval dataset list` shows the ones there are", dataset, version)
+			"`azd ai eval dataset versions list %s` shows the ones there are", dataset, version, dataset)
 }
 
 // DatasetVersionNotFound reports a dataset version there is nothing to delete at.

cli/azd/extensions/azure.ai.evaluations/tests/cli/run_output_test.go

@@ -192,7 +192,7 @@ func TestCLIResultsExport(t *testing.T) {
 		require.NoError(t, err, "--format csv must emit parseable CSV:\n%s", r.Stdout)
 		require.Len(t, rows, 2, "a header and one row per criterion")
 		require.Equal(t,
-			[]string{"run_id", "status", "criterion", "passed", "failed"}, rows[0])
+			[]string{"run_id", "status", "testing_criteria", "passed", "failed"}, rows[0])
 		require.Equal(t, f.FirstRunID, rows[1][0])
 		require.Equal(t, "completed", rows[1][1])
 		require.Equal(t, f.EvaluatorName, rows[1][2])