From 7a8632d46f3adb68e7618ab5dca01332e1450248 Mon Sep 17 00:00:00 2001 From: Rob Zolkos Date: Fri, 2 Oct 2026 14:55:19 -0400 Subject: [PATCH] Run the connector's end-to-end scenarios against a local Basecamp too The scenarios in e2e/connect ran only against the fake Basecamp. Nothing showed that the fake behaves as Basecamp does. Now the same scenarios also run against a local BC3, so passing on both is the conformance check between the two. - Target says what a scenario needs in its own words: the CLI to run, the serving agent, the operator mentions the agent, an untrusted person mentions the agent. Posted is the comment as Basecamp holds it, so the request line is checked field by field on either target. - fakeTarget is the earlier behavior through fakebasecamp. - devTarget drives a local Basecamp with the CLI under test and the developer's own config and credentials. It reads connect.json with `connect show` and never runs setup. Each scenario gets a temporary XDG_STATE_HOME (its own ledger and instance lock), a new message posted by the operator with nobody subscribed, and comments made with `comments create` and `[@agent](person:)`. It trashes the message at the end. It refuses to run while `connect status` shows a connector running for the agent, and it refuses a BASECAMP_BASE_URL that is not local. - These run on both targets: a trusted mention, an untrusted mention, stdout only NDJSON, SIGINT, a second connector refused, and a request handed off after --hold, kill -9, release and restart. Scenarios that need a fault only the fake can make call fakeOnly with their reason. The dev run shows them as skipped. - `make test-connect-dev` sets BASECAMP_CONNECT_DEV=1 and fails when a required variable is missing. A plain `go test` runs only the fake. - CONTRIBUTING.md tells how to run it and what it covers. --- CONTRIBUTING.md | 73 +++++ Makefile | 20 ++ e2e/connect/connect_test.go | 160 +++++----- e2e/connect/connector_test.go | 2 +- e2e/connect/dev_target_test.go | 500 ++++++++++++++++++++++++++++++++ e2e/connect/failures_test.go | 91 +++--- e2e/connect/fake_target_test.go | 67 +++++ e2e/connect/harness_test.go | 31 +- e2e/connect/main_test.go | 9 +- e2e/connect/target_test.go | 152 ++++++++++ 10 files changed, 981 insertions(+), 124 deletions(-) create mode 100644 e2e/connect/dev_target_test.go create mode 100644 e2e/connect/fake_target_test.go create mode 100644 e2e/connect/target_test.go diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7b29bb7e4..c2293d4af 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -103,6 +103,79 @@ make test-e2e BASECAMP_BIN=./bin/basecamp bats e2e/ ``` +### Running basecamp connect against a local Basecamp + +The connector's end-to-end scenarios in `e2e/connect` run the real binary +against the fake Basecamp in `internal/connector/fakebasecamp` as part of +`make test`. The same scenarios also run against a local Basecamp (BC3 in +development). Passing on both is the check that the fake behaves as Basecamp +does. + +Prerequisites, all against the local Basecamp: + +1. The event feed is enabled for the account, and Action Cable is served, so + `basecamp connect` can stream. +2. An operator profile, logged in as you: + `basecamp profile create rob --base-url http://3.basecamp.localhost:3001`. +3. An untrusted profile, logged in as another person on the project. The + agent takes no instructions from this person. +4. An agent connected under a profile of its own and set up in operator trust + (the default) to serve the project: + + ```bash + basecamp auth agent connect -P agent + basecamp connect setup -P agent --operator-profile rob --serve + ``` + +5. No connector running for that agent. The scenarios stop with an error if + `basecamp connect status -P agent` shows one, because that connector would + get the scenarios' mentions too. + +Then run: + +```bash +BASECAMP_BASE_URL=http://3.basecamp.localhost:3001 \ +BASECAMP_CONNECT_DEV_AGENT_PROFILE=agent \ +BASECAMP_CONNECT_DEV_OPERATOR_PROFILE=rob \ +BASECAMP_CONNECT_DEV_UNTRUSTED_PROFILE=jason \ +BASECAMP_CONNECT_DEV_PROJECT_ID= \ +make test-connect-dev +``` + +The target fails, and does not skip, when a variable is missing. It also +refuses a `BASECAMP_BASE_URL` that is not `localhost`, a loopback address or a +`*.localhost` host, because the scenarios post as your profiles. + +What it does: + +- It uses your config and your credentials (keyring included) as they are. + It never runs `auth agent connect` or `connect setup`, and it does not + change `connect.json`. It reads `connect.json` with `basecamp connect show`. +- Each scenario gives its connector a temporary `XDG_STATE_HOME`. The + connector's ledger and its per-agent instance lock are there. They are not + yours, so the scenario cannot read or move your connector's position, and + it cannot hold your lock. +- Each scenario posts a new message in the project as the operator, with + nobody subscribed. It comments on that message with `basecamp comments + create`, and mentions the agent as `[@agent](person:)`. When the + scenario ends, it trashes the message and its comments. +- The scenarios run one at a time. + +What it covers: a trusted mention handed off in one request line (field by +field, from the comment Basecamp returned), an untrusted mention discarded as +`untrusted_performer`, stdout that holds only NDJSON, a clean SIGINT, a +second connector for the same agent refused at once, and a request admitted +under `--hold` that is handed off after a `kill -9`, `connect release` and a +restart. + +What stays fake-only: the scenarios that need a fault or a world only the +fake can make. These are a dropped or silent live connection, the poll lane +and the live lane published apart, a rate-limited token endpoint, failed or +held recording reads, an agent disconnected or connected on another computer, +tokens that last a second, a second agent, and `connect setup` itself. Each +one calls `fakeOnly` with its reason. The run against the local Basecamp +shows them as skipped, with that reason. + ## Questions? Open an issue for questions about contributing. diff --git a/Makefile b/Makefile index 203d16f31..e0d2a8be5 100644 --- a/Makefile +++ b/Makefile @@ -79,6 +79,25 @@ test: check-toolchain test-e2e: build ./e2e/run.sh +# Run the connector's end-to-end scenarios (e2e/connect) against a local +# Basecamp instead of the fake: the conformance check between the two. +# Scenarios that need a fault only the fake can produce are skipped, by name. +# See CONTRIBUTING.md, "Running basecamp connect against a local Basecamp". +# Required env vars: +# BASECAMP_BASE_URL - the local Basecamp (e.g. http://3.basecamp.localhost:3001) +# BASECAMP_CONNECT_DEV_AGENT_PROFILE - an agent connected and set up to serve the project +# BASECAMP_CONNECT_DEV_OPERATOR_PROFILE - the agent's operator, logged in +# BASECAMP_CONNECT_DEV_UNTRUSTED_PROFILE - someone else on the project, logged in +# BASECAMP_CONNECT_DEV_PROJECT_ID - the served project the scenarios post in +.PHONY: test-connect-dev +test-connect-dev: check-toolchain + @test -n "$(BASECAMP_BASE_URL)" || { echo "Set BASECAMP_BASE_URL (e.g. http://3.basecamp.localhost:3001); see CONTRIBUTING.md"; exit 1; } + @test -n "$(BASECAMP_CONNECT_DEV_AGENT_PROFILE)" || { echo "Set BASECAMP_CONNECT_DEV_AGENT_PROFILE (an agent connected and set up to serve the project); see CONTRIBUTING.md"; exit 1; } + @test -n "$(BASECAMP_CONNECT_DEV_OPERATOR_PROFILE)" || { echo "Set BASECAMP_CONNECT_DEV_OPERATOR_PROFILE (the agent's operator, logged in); see CONTRIBUTING.md"; exit 1; } + @test -n "$(BASECAMP_CONNECT_DEV_UNTRUSTED_PROFILE)" || { echo "Set BASECAMP_CONNECT_DEV_UNTRUSTED_PROFILE (someone else on the project, logged in); see CONTRIBUTING.md"; exit 1; } + @test -n "$(BASECAMP_CONNECT_DEV_PROJECT_ID)" || { echo "Set BASECAMP_CONNECT_DEV_PROJECT_ID (the served project the scenarios post in); see CONTRIBUTING.md"; exit 1; } + BASECAMP_CONNECT_DEV=1 $(GOTEST) -count=1 -failfast -v ./e2e/connect + # Record cassettes for happy-path replay tests. # Required env vars: # BASECAMP_RECORD_TOKEN - valid API token for the target server @@ -625,6 +644,7 @@ help: @echo " race-test Run tests with race detector" @echo " test-coverage Run tests with coverage report" @echo " coverage Run tests with coverage and open in browser" + @echo " test-connect-dev Run the connector's e2e scenarios against a local Basecamp (see CONTRIBUTING.md)" @echo " record-cassettes Record happy-path cassettes (TOKEN+TARGET+ACCOUNT+PROJECT)" @echo " smoke Run pre-release smoke suite (BASECAMP_TOKEN=...)" @echo " check-race-shards Test the race shards' union check" diff --git a/e2e/connect/connect_test.go b/e2e/connect/connect_test.go index 0b94f5e37..39c30c714 100644 --- a/e2e/connect/connect_test.go +++ b/e2e/connect/connect_test.go @@ -64,17 +64,15 @@ func askAgent() string { // event published live reaches it. func streaming(t *testing.T) (*Harness, *Connector) { t.Helper() - h := NewHarness(t, world()) - h.Setup(t) - c := h.Connect(t) - c.WaitStreaming(t, 1) - return h, c + f := newFakeTarget(t) + return f.h, streamingOn(t, f) } // The setup a person runs, through the CLI: the connection ceremony, then // connect setup, which writes connect.json owner-only. func TestSetupThroughTheCLIWritesConnectJSON(t *testing.T) { t.Parallel() + fakeOnly(t, "it runs setup, which a local Basecamp's developer has run already, and connect.json there is theirs") h := NewHarness(t, world()) h.Setup(t) @@ -114,41 +112,47 @@ func TestSetupThroughTheCLIWritesConnectJSON(t *testing.T) { // mention taken out of the words. func TestATrustedMentionIsHandedOffInOneRequestLine(t *testing.T) { t.Parallel() - h, c := streaming(t) - ev := comment(h, fakebasecamp.OperatorID, askAgent(), fakebasecamp.Live) - c.WaitFor(t, "the request line", func(o Output) bool { return len(o.RequestsFor(ev.ID)) > 0 }) - out := c.Stop(t) + onEachTarget(t, func(t *testing.T, tg Target) { + c := streamingOn(t, tg) + p := tg.OperatorMentionsAgent(t) + c.WaitFor(t, "the request line", func(o Output) bool { return len(o.RequestsOn(p)) > 0 }) + out := c.Stop(t) - pointers := out.PointersFor(ev.ID) - require.Len(t, pointers, 1) - assert.Equal(t, "live", pointers[0].Lane) - assert.Equal(t, ev.RecordingID, pointers[0].RecordingID) - - events := out.EventsFor(ev.ID) - require.Len(t, events, 1) - assert.Equal(t, "admitted", events[0].State) - assert.Equal(t, "mentioned", events[0].Trigger) - - requests := out.RequestsFor(ev.ID) - require.Len(t, requests, 1) - req := requests[0] - assert.Equal(t, "request", req.Type) - assert.Equal(t, "comment.created", req.EventType) - assert.Equal(t, "mentioned", req.Trigger) - assert.Equal(t, RequestRecording{ - BucketID: fakebasecamp.ProjectID, - ProjectName: fakebasecamp.ProjectName, - RecordingID: ev.RecordingID, - Type: "Comment", - URL: h.Fake.URL() + "/" + strconv.FormatInt(fakebasecamp.AccountID, 10) + "/comments/" + strconv.FormatInt(ev.RecordingID, 10), - }, req.Recording) - assert.Equal(t, RequestReplyTo{Kind: "comment", RecordingID: kickoffID}, req.ReplyTo) - assert.Equal(t, fakebasecamp.OperatorID, req.RequesterID) - assert.Equal(t, fakebasecamp.OperatorName, req.RequesterName) - assert.True(t, req.Acknowledge, "a person asked for something") - // The mention leaves a space where it was, so no markup can close over - // the gap, and nothing else of the words changes. - assert.Equal(t, strings.Replace(askAgent(), fakebasecamp.Mention(fakebasecamp.AgentID), " ", 1), req.Content) + pointers := out.PointersOn(p) + require.Len(t, pointers, 1) + assert.Equal(t, "live", pointers[0].Lane, "Basecamp publishes live first") + assert.Equal(t, p.BucketID, pointers[0].BucketID) + assert.Equal(t, p.AuthorID, pointers[0].CreatorID) + + events := out.EventsOn(p) + require.Len(t, events, 1) + assert.Equal(t, pointers[0].EventID, events[0].EventID) + assert.Equal(t, "admitted", events[0].State) + assert.Equal(t, "mentioned", events[0].Trigger) + assert.Equal(t, p.URL, events[0].RecordingURL) + + requests := out.RequestsOn(p) + require.Len(t, requests, 1) + req := requests[0] + assert.Equal(t, pointers[0].EventID, req.EventID) + assert.Equal(t, "request", req.Type) + assert.Equal(t, "mentioned", req.Trigger) + assert.Equal(t, RequestRecording{ + BucketID: p.BucketID, + ProjectName: p.ProjectName, + RecordingID: p.RecordingID, + Type: "Comment", + Title: p.Title, + URL: p.URL, + }, req.Recording) + assert.Equal(t, RequestReplyTo{Kind: "comment", RecordingID: p.ParentID}, req.ReplyTo) + assert.Equal(t, p.AuthorID, req.RequesterID) + assert.Equal(t, p.AuthorName, req.RequesterName) + assert.True(t, req.Acknowledge, "a person asked for something") + // The mention leaves a space where it was, so no markup can close over + // the gap, and nothing else of the words changes. + assert.Equal(t, p.Instruction, req.Content) + }) } // Basecamp publishes an event on both lanes: live first, and on the poll @@ -156,6 +160,7 @@ func TestATrustedMentionIsHandedOffInOneRequestLine(t *testing.T) { // one pointer line and one request line. func TestAnEventOnBothLanesIsHandedOffOnce(t *testing.T) { t.Parallel() + fakeOnly(t, "it decides what each lane serves and drops the live connection") h, c := streaming(t) ev := comment(h, fakebasecamp.OperatorID, askAgent(), fakebasecamp.Both) c.WaitFor(t, "the request line", func(o Output) bool { return len(o.RequestsFor(ev.ID)) > 0 }) @@ -188,18 +193,20 @@ func TestAnEventOnBothLanesIsHandedOffOnce(t *testing.T) { // connector says so in an event line, and hands nothing off. func TestAnUntrustedMentionIsDiscardedAndNotHandedOff(t *testing.T) { t.Parallel() - h, c := streaming(t) - ev := comment(h, BystanderID, askAgent(), fakebasecamp.Live) - c.WaitFor(t, "the event line", func(o Output) bool { return len(o.EventsFor(ev.ID)) > 0 }) - out := c.Stop(t) + onEachTarget(t, func(t *testing.T, tg Target) { + c := streamingOn(t, tg) + p := tg.UntrustedMentionsAgent(t) + c.WaitFor(t, "the event line", func(o Output) bool { return len(o.EventsOn(p)) > 0 }) + out := c.Stop(t) - events := out.EventsFor(ev.ID) - require.Len(t, events, 1) - assert.Equal(t, "discarded", events[0].State) - assert.Equal(t, "untrusted_performer", events[0].Reason) - assert.Equal(t, BystanderID, events[0].RequesterID) - assert.Empty(t, out.RequestsFor(ev.ID)) - assert.Empty(t, out.Requests()) + events := out.EventsOn(p) + require.Len(t, events, 1) + assert.Equal(t, "discarded", events[0].State) + assert.Equal(t, "untrusted_performer", events[0].Reason) + assert.Equal(t, p.AuthorID, events[0].RequesterID) + assert.Empty(t, out.RequestsOn(p)) + assert.Empty(t, out.RequestsAbout(p), "nothing on the recording is handed off") + }) } // Stdout is a protocol a program parses: every line one JSON object of a @@ -207,32 +214,35 @@ func TestAnUntrustedMentionIsDiscardedAndNotHandedOff(t *testing.T) { // stderr, as slog lines, starting with the connector saying it runs. func TestStdoutIsOnlyNDJSONAndStderrSaysItRuns(t *testing.T) { t.Parallel() - h, c := streaming(t) - trusted := comment(h, fakebasecamp.OperatorID, askAgent(), fakebasecamp.Live) - untrusted := comment(h, BystanderID, askAgent(), fakebasecamp.Live) - c.WaitFor(t, "both verdicts and the request", func(o Output) bool { - return len(o.RequestsFor(trusted.ID)) > 0 && len(o.EventsFor(untrusted.ID)) > 0 - }) - out := c.Stop(t) + onEachTarget(t, func(t *testing.T, tg Target) { + c := streamingOn(t, tg) + trusted := tg.OperatorMentionsAgent(t) + untrusted := tg.UntrustedMentionsAgent(t) + c.WaitFor(t, "both verdicts and the request", func(o Output) bool { + return len(o.RequestsOn(trusted)) > 0 && len(o.EventsOn(untrusted)) > 0 + }) + out := c.Stop(t) - require.NotEmpty(t, out.Lines) - for _, l := range out.Lines { - assert.True(t, json.Valid([]byte(l.Raw)), "not JSON: %q", l.Raw) - assert.NoError(t, l.Err, "%q", l.Raw) - } - assert.NotEmpty(t, out.Pointers()) - assert.NotEmpty(t, out.Events()) - assert.NotEmpty(t, out.Requests()) - - running := out.Logged(MsgRunning) - require.Len(t, running, 1) - assert.Equal(t, "INFO", running[0].Level) - assert.Equal(t, Agent, running[0].Attrs["profile"]) - assert.Equal(t, strconv.FormatInt(fakebasecamp.AccountID, 10), running[0].Attrs["account"]) - assert.Equal(t, strconv.FormatInt(fakebasecamp.AgentID, 10), running[0].Attrs["agent_person_id"]) - for _, l := range out.Logs { - assert.NotEmpty(t, l.Msg, "stderr line is not a slog record: %q", l.Raw) - } + require.NotEmpty(t, out.Lines) + for _, l := range out.Lines { + assert.True(t, json.Valid([]byte(l.Raw)), "not JSON: %q", l.Raw) + assert.NoError(t, l.Err, "%q", l.Raw) + } + assert.NotEmpty(t, out.Pointers()) + assert.NotEmpty(t, out.Events()) + assert.NotEmpty(t, out.Requests()) + + agent := tg.Agent() + running := out.Logged(MsgRunning) + require.Len(t, running, 1) + assert.Equal(t, "INFO", running[0].Level) + assert.Equal(t, agent.Profile, running[0].Attrs["profile"]) + assert.Equal(t, strconv.FormatInt(agent.AccountID, 10), running[0].Attrs["account"]) + assert.Equal(t, strconv.FormatInt(agent.PersonID, 10), running[0].Attrs["agent_person_id"]) + for _, l := range out.Logs { + assert.NotEmpty(t, l.Msg, "stderr line is not a slog record: %q", l.Raw) + } + }) } // cursorBefore reports whether a poll position the fake issued is before diff --git a/e2e/connect/connector_test.go b/e2e/connect/connector_test.go index 0ac9246c3..00a988bac 100644 --- a/e2e/connect/connector_test.go +++ b/e2e/connect/connector_test.go @@ -72,7 +72,7 @@ func (h *Harness) Connect(t *testing.T, extra ...string) *Connector { // flags, in the harness's home. func (h *Harness) ConnectProfile(t *testing.T, p Profile, extra ...string) *Connector { t.Helper() - return h.Start(t, h.Home, append([]string{"connect", "-P", p.Name}, extra...)...) + return h.Start(t, h.Dir, append([]string{"connect", "-P", p.Name}, extra...)...) } // Start starts a CLI command in the background, in dir, with the harness's diff --git a/e2e/connect/dev_target_test.go b/e2e/connect/dev_target_test.go new file mode 100644 index 000000000..344b0a655 --- /dev/null +++ b/e2e/connect/dev_target_test.go @@ -0,0 +1,500 @@ +//go:build linux || darwin + +package connect + +import ( + "context" + "encoding/json" + "fmt" + "net/url" + "os" + "path/filepath" + "regexp" + "strconv" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/basecamp/basecamp-cli/internal/connector/fakebasecamp" + "github.com/basecamp/basecamp-cli/internal/hostutil" +) + +// The environment devTarget reads. `make test-connect-dev` sets envDev and +// requires the rest; CONTRIBUTING.md says how to get each one. +const ( + // envDev, set to 1, runs the scenarios against the local Basecamp + // instead of the fake, and skips those only the fake can run. + envDev = "BASECAMP_CONNECT_DEV" + // envBaseURL is the CLI's own: the local Basecamp every command talks to. + envBaseURL = "BASECAMP_BASE_URL" + // envAgentProfile is the profile of an agent already connected and set + // up with `basecamp connect setup`, in operator trust mode, serving + // envProjectID. + envAgentProfile = "BASECAMP_CONNECT_DEV_AGENT_PROFILE" + // envOperatorProfile is a profile logged in as the agent's operator: + // the person connect.json trusts. + envOperatorProfile = "BASECAMP_CONNECT_DEV_OPERATOR_PROFILE" + // envUntrustedProfile is a profile logged in as someone else on the + // project, whom the agent takes no instructions from. + envUntrustedProfile = "BASECAMP_CONNECT_DEV_UNTRUSTED_PROFILE" + // envProjectID is the served project the scenarios post in. + envProjectID = "BASECAMP_CONNECT_DEV_PROJECT_ID" +) + +// contributing is where the dev target's prerequisites are written down. +const contributing = `CONTRIBUTING.md, "Running basecamp connect against a local Basecamp"` + +// devRequested says `make test-connect-dev` asked for the local Basecamp. +func devRequested() bool { return os.Getenv(envDev) == "1" } + +// devSerial runs the scenarios against the local Basecamp one at a time. +var devSerial sync.Mutex + +// fakeOnly marks a scenario that runs only against the fake, and says why: +// it needs a fault or a world only the fake can produce. Against the local +// Basecamp it is skipped, so the run lists what it did not cover. +func fakeOnly(t *testing.T, why string) { + t.Helper() + if devRequested() { + t.Skip("fake only: " + why) + } +} + +// devConfig is the dev target's environment, read and checked. +type devConfig struct { + BaseURL string + AgentProfile string + OperatorProfile string + UntrustedProfile string + ProjectID int64 +} + +// devConfigFrom reads the dev target's environment through getenv. It +// names every variable that is missing at once, and refuses a Basecamp +// that is not local: the scenarios post and trash messages as the +// developer's profiles. +func devConfigFrom(getenv func(string) string) (devConfig, error) { + var missing []string + get := func(key string) string { + v := strings.TrimSpace(getenv(key)) + if v == "" { + missing = append(missing, key) + } + return v + } + c := devConfig{ + BaseURL: get(envBaseURL), + AgentProfile: get(envAgentProfile), + OperatorProfile: get(envOperatorProfile), + UntrustedProfile: get(envUntrustedProfile), + } + project := get(envProjectID) + if len(missing) > 0 { + return devConfig{}, fmt.Errorf("the local Basecamp target needs %s set; see %s", strings.Join(missing, ", "), contributing) + } + + id, err := strconv.ParseInt(project, 10, 64) + if err != nil || id <= 0 { + return devConfig{}, fmt.Errorf("%s=%q is not a project id", envProjectID, project) + } + c.ProjectID = id + + u, err := url.Parse(c.BaseURL) + if err != nil || (u.Scheme != "http" && u.Scheme != "https") || u.Host == "" { + return devConfig{}, fmt.Errorf("%s=%q is not an http(s) URL", envBaseURL, c.BaseURL) + } + if !hostutil.IsLocalhost(u.Host) { + return devConfig{}, fmt.Errorf("%s=%q is not a local Basecamp: the scenarios post and trash messages as your profiles, so they run only against localhost or a *.localhost host", envBaseURL, c.BaseURL) + } + + if c.AgentProfile == c.OperatorProfile || c.AgentProfile == c.UntrustedProfile || c.OperatorProfile == c.UntrustedProfile { + return devConfig{}, fmt.Errorf("%s, %s and %s must name three different profiles", envAgentProfile, envOperatorProfile, envUntrustedProfile) + } + return c, nil +} + +// devEnv is the environment the CLI gets against the local Basecamp: the +// developer's own, so their config and credentials (keyring included) are +// used as they are, less what would change who a command acts as or where. +// A BASECAMP_TOKEN would stand in for every profile's credential (and the +// connector refuses to run under one); a default profile, account, project +// or to-do list would steer a command the scenarios name exactly. +// +// stateHome, when set, is the connector's state for the test alone: its +// ledger and its instance lock, which live under XDG_STATE_HOME. A +// connector the developer runs for the same agent keeps its own, so a test +// can neither read its ledger nor hold its lock. Empty leaves the +// developer's. +func devEnv(environ []string, stateHome string) []string { + drop := map[string]bool{ + "BASECAMP_TOKEN": true, + "BASECAMP_PROFILE": true, + "BASECAMP_ACCOUNT_ID": true, + "BASECAMP_PROJECT_ID": true, + "BASECAMP_TODOLIST_ID": true, + "BASECAMP_NO_UPDATE_CHECK": true, + } + if stateHome != "" { + drop["XDG_STATE_HOME"] = true + } + env := make([]string, 0, len(environ)+2) + for _, kv := range environ { + key, _, _ := strings.Cut(kv, "=") + if !drop[key] { + env = append(env, kv) + } + } + env = append(env, "BASECAMP_NO_UPDATE_CHECK=1") + if stateHome != "" { + env = append(env, "XDG_STATE_HOME="+stateHome) + } + return env +} + +// devTarget is a local Basecamp as a Target, driven with the CLI under +// test and the developer's own profiles. +// +// It assumes the agent is connected and set up already (`basecamp auth +// agent connect` and `basecamp connect setup`), and checks that setup with +// `basecamp connect show` rather than running it: setup rewrites +// connect.json, which is the developer's. Each scenario gets a message of +// its own in the served project, posted by the operator with nobody else +// subscribed, and trashed, comments and all, when the scenario ends. Its +// connector runs under a state directory of its own (see devEnv), and it +// refuses to start while a connector for the same agent is running from the +// developer's state: that connector would be handed the scenario's +// mentions too. +type devTarget struct { + h *Harness + cfg devConfig + agent ServingAgent + // operatorID is the person connect.json trusts. + operatorID int64 + // message is the scenario's message, which every comment is on. + message int64 +} + +func newDevTarget(t *testing.T) *devTarget { + t.Helper() + cfg, err := devConfigFrom(os.Getenv) + require.NoError(t, err) + dir := t.TempDir() + // The working directory is a fresh one, so no repository's + // .basecamp/config.json is read. + d := &devTarget{ + h: &Harness{t: t, Dir: dir, env: devEnv(os.Environ(), filepath.Join(dir, "state"))}, + cfg: cfg, + } + + setup := d.connectShow(t) + d.agent = ServingAgent{Profile: cfg.AgentProfile, PersonID: setup.Agent.PersonID, AccountID: setup.AccountID} + d.operatorID = setup.Trust.OperatorID + d.requireNoRunningConnector(t) + + var msg struct { + ID int64 `json:"id"` + Creator devPerson `json:"creator"` + } + d.as(t, cfg.OperatorProfile, &msg, "messages", "create", + "--project", strconv.FormatInt(cfg.ProjectID, 10), "--no-subscribe", + "connect e2e "+t.Name()+" "+time.Now().UTC().Format(time.RFC3339), + "A message the connector's end-to-end scenarios comment on. It is trashed when the scenario ends.") + require.NotZero(t, msg.ID, "the message has no id") + d.message = msg.ID + t.Cleanup(func() { d.trash(t) }) + require.Equal(t, d.operatorID, msg.Creator.ID, + "%s is person %d, not the operator connect.json trusts (%d)", envOperatorProfile, msg.Creator.ID, d.operatorID) + return d +} + +func (d *devTarget) CLI() *Harness { return d.h } + +func (d *devTarget) Agent() ServingAgent { return d.agent } + +func (d *devTarget) OperatorMentionsAgent(t *testing.T) Posted { + t.Helper() + return d.post(t, d.cfg.OperatorProfile) +} + +func (d *devTarget) UntrustedMentionsAgent(t *testing.T) Posted { + t.Helper() + p := d.post(t, d.cfg.UntrustedProfile) + require.NotEqual(t, d.operatorID, p.AuthorID, "%s is the operator", envUntrustedProfile) + return p +} + +// post has profile comment on the scenario's message, mentioning the agent +// by its person id as the CLI's Markdown writes a mention, and returns the +// comment as Basecamp answered the create: the same representation the +// connector reads. +func (d *devTarget) post(t *testing.T, profile string) Posted { + t.Helper() + var c struct { + ID int64 `json:"id"` + Type string `json:"type"` + Title string `json:"title"` + AppURL string `json:"app_url"` + Content string `json:"content"` + Parent struct { + ID int64 `json:"id"` + } `json:"parent"` + Bucket struct { + ID int64 `json:"id"` + Name string `json:"name"` + } `json:"bucket"` + Creator devPerson `json:"creator"` + } + ask := "Could you review the launch plan, [@agent](person:" + strconv.FormatInt(d.agent.PersonID, 10) + ")? Thanks" + d.as(t, profile, &c, "comments", "create", strconv.FormatInt(d.message, 10), ask) + instruction, err := withoutMention(c.Content) + require.NoError(t, err, "the comment as Basecamp holds it: %q", c.Content) + return Posted{ + RecordingID: c.ID, + ParentID: c.Parent.ID, + BucketID: c.Bucket.ID, + ProjectName: c.Bucket.Name, + Type: c.Type, + Title: c.Title, + URL: c.AppURL, + AuthorID: c.Creator.ID, + AuthorName: c.Creator.Name, + Instruction: instruction, + } +} + +// mentionElement is one mention attachment, start tag to end tag, as +// Basecamp writes it into rich text. +var mentionElement = regexp.MustCompile(`(?s)]*\bcontent-type="application/vnd\.basecamp\.mention"[^>]*>.*?`) + +// withoutMention is content with its one mention element replaced by a +// space, which is what the connector hands off of a comment that mentions +// only the agent. It is worked out here from the markup, not by the +// connector's own code, so the two can disagree. +func withoutMention(content string) (string, error) { + found := mentionElement.FindAllStringIndex(content, -1) + if len(found) != 1 { + return "", fmt.Errorf("want one mention in the comment, found %d", len(found)) + } + return content[:found[0][0]] + " " + content[found[0][1]:], nil +} + +// devPerson is a person as a Basecamp record names them. +type devPerson struct { + ID int64 `json:"id"` + Name string `json:"name"` +} + +// devSetup is the part of connect.json the scenarios depend on. +type devSetup struct { + AccountID int64 `json:"-"` + Agent struct { + PersonID int64 `json:"person_id"` + } `json:"agent"` + Trust struct { + Mode string `json:"mode"` + OperatorID int64 `json:"operator_id"` + } `json:"trust"` + Projects map[string]json.RawMessage `json:"projects"` +} + +// connectShow reads the agent's setup and requires what the scenarios +// depend on: operator trust, so the untrusted profile is untrusted, and the +// project served. +func (d *devTarget) connectShow(t *testing.T) devSetup { + t.Helper() + var file struct { + devSetup + AccountID string `json:"account_id"` + } + d.data(t, d.h.MustRun(t, "connect", "show", "-P", d.cfg.AgentProfile, "--json"), &file) + s := file.devSetup + var err error + s.AccountID, err = strconv.ParseInt(file.AccountID, 10, 64) + require.NoError(t, err, "connect.json's account_id %q", file.AccountID) + require.Equal(t, "operator", s.Trust.Mode, + "%s is set up in %q trust mode; the scenarios need operator trust, setup's default", envAgentProfile, s.Trust.Mode) + require.Contains(t, s.Projects, strconv.FormatInt(d.cfg.ProjectID, 10), + "%s does not serve project %d; serve it with: basecamp connect setup -P %s --serve %d", envAgentProfile, d.cfg.ProjectID, d.cfg.AgentProfile, d.cfg.ProjectID) + return s +} + +// requireNoRunningConnector refuses to go on while a connector for the +// agent runs from the developer's own state, which `connect status` reads +// without taking its lock. A profile whose connector has never run has no +// ledger, and status says so. +func (d *devTarget) requireNoRunningConnector(t *testing.T) { + t.Helper() + own := &Harness{t: t, Dir: d.h.Dir, env: devEnv(os.Environ(), "")} + res := own.Run(t, "connect", "status", "-P", d.cfg.AgentProfile, "--json") + if res.ExitCode != 0 { + require.Contains(t, res.Stdout+res.Stderr, "has no ledger yet", "basecamp connect status -P %s\n%s%s", d.cfg.AgentProfile, res.Stdout, res.Stderr) + return + } + var status struct { + LockHolder *struct { + PID int `json:"pid"` + PIDStatus string `json:"pid_status"` + } `json:"lock_holder"` + } + d.data(t, res, &status) + if status.LockHolder != nil && status.LockHolder.PIDStatus == "present" { + t.Fatalf("a connector for %s may be running (pid %d): stop it first, or it is handed the scenarios' mentions too", d.cfg.AgentProfile, status.LockHolder.PID) + } +} + +// as runs a command as profile in the agent's account, requires that it +// succeeds, and decodes its JSON data into v. +func (d *devTarget) as(t *testing.T, profile string, v any, args ...string) { + t.Helper() + args = append(args, "-P", profile, "--account", strconv.FormatInt(d.agent.AccountID, 10), "--json") + d.data(t, d.h.MustRun(t, args...), v) +} + +// trash trashes the scenario's message, and with it every comment on it. +// It runs as a cleanup, after the test's context has ended. +func (d *devTarget) trash(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), waitFor) + defer cancel() + res := d.h.run(ctx, t, "messages", "trash", strconv.FormatInt(d.message, 10), + "-P", d.cfg.OperatorProfile, "--account", strconv.FormatInt(d.agent.AccountID, 10), "--json") + if res.ExitCode != 0 { + t.Errorf("trash the scenario's message %d: exit %d\n%s%s", d.message, res.ExitCode, res.Stdout, res.Stderr) + } +} + +// data decodes a command's JSON envelope's data into v. +func (d *devTarget) data(t *testing.T, res Result, v any) { + t.Helper() + var envelope struct { + Data json.RawMessage `json:"data"` + } + require.NoError(t, json.Unmarshal([]byte(res.Stdout), &envelope), res.Stdout) + require.NotEmpty(t, envelope.Data, "no data: %s", res.Stdout) + require.NoError(t, json.Unmarshal(envelope.Data, v), res.Stdout) +} + +// The dev target refuses to start on an environment it cannot run in, and +// says what is wrong with it, all of it at once. +func TestDevConfigNamesEveryMissingVariable(t *testing.T) { + _, err := devConfigFrom(envOf(map[string]string{envAgentProfile: "agent"})) + require.Error(t, err) + for _, key := range []string{envBaseURL, envOperatorProfile, envUntrustedProfile, envProjectID} { + assert.Contains(t, err.Error(), key) + } + assert.NotContains(t, err.Error(), envAgentProfile) + assert.Contains(t, err.Error(), "CONTRIBUTING.md") + + _, err = devConfigFrom(envOf(nil)) + require.Error(t, err) + assert.Contains(t, err.Error(), envAgentProfile) +} + +func TestDevConfigReadsAValidEnvironment(t *testing.T) { + c, err := devConfigFrom(envOf(validDevEnv())) + require.NoError(t, err) + assert.Equal(t, devConfig{ + BaseURL: "http://3.basecamp.localhost:3001", + AgentProfile: "agent", + OperatorProfile: "rob", + UntrustedProfile: "jason", + ProjectID: 2085958499, + }, c) +} + +func TestDevConfigRefusesWhatIsNotAProjectOrALocalBasecamp(t *testing.T) { + for _, project := range []string{"Launch", "0", "-4", "12x"} { + env := validDevEnv() + env[envProjectID] = project + _, err := devConfigFrom(envOf(env)) + assert.ErrorContains(t, err, "not a project id", "%q", project) + } + for _, base := range []string{"https://3.basecampapi.com", "http://basecamp.example", "3.basecamp.localhost:3001", "ftp://localhost"} { + env := validDevEnv() + env[envBaseURL] = base + _, err := devConfigFrom(envOf(env)) + assert.ErrorContains(t, err, envBaseURL, "%q", base) + } + for _, base := range []string{"http://localhost:3001", "http://127.0.0.1:3001", "http://[::1]:3001", "https://3.basecamp.localhost"} { + env := validDevEnv() + env[envBaseURL] = base + _, err := devConfigFrom(envOf(env)) + assert.NoError(t, err, "%q", base) + } + env := validDevEnv() + env[envUntrustedProfile] = env[envOperatorProfile] + _, err := devConfigFrom(envOf(env)) + assert.ErrorContains(t, err, "three different profiles") +} + +// The CLI against a local Basecamp gets the developer's environment, less +// what would change who a command acts as, with the connector's state the +// test's own. +func TestDevEnvKeepsTheDevelopersCredentialsAndIsolatesTheConnectorState(t *testing.T) { + environ := []string{ + "HOME=/home/dev", + "XDG_CONFIG_HOME=/home/dev/.config", + "XDG_STATE_HOME=/home/dev/.local/state", + "DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1000/bus", + "BASECAMP_BASE_URL=http://3.basecamp.localhost:3001", + "BASECAMP_TOKEN=secret", + "BASECAMP_PROFILE=rob", + "BASECAMP_ACCOUNT_ID=1", + "BASECAMP_PROJECT_ID=2", + "BASECAMP_TODOLIST_ID=3", + "BASECAMP_NO_UPDATE_CHECK=0", + } + assert.Equal(t, []string{ + "HOME=/home/dev", + "XDG_CONFIG_HOME=/home/dev/.config", + "DBUS_SESSION_BUS_ADDRESS=unix:path=/run/user/1000/bus", + "BASECAMP_BASE_URL=http://3.basecamp.localhost:3001", + "BASECAMP_NO_UPDATE_CHECK=1", + "XDG_STATE_HOME=/tmp/test/state", + }, devEnv(environ, "/tmp/test/state")) + + own := devEnv(environ, "") + assert.Contains(t, own, "XDG_STATE_HOME=/home/dev/.local/state", "the developer's own state, to look for their connector") + assert.NotContains(t, own, "BASECAMP_TOKEN=secret") +} + +// The instruction a request line should carry is worked out from the +// comment's markup: its one mention, start tag to end tag, becomes a space, +// whatever Basecamp renders inside it. +func TestWithoutMentionTakesOutTheOneMention(t *testing.T) { + got, err := withoutMention(`

Could you review the launch plan, ` + + `` + + `

Chef
? Thanks

`) + require.NoError(t, err) + assert.Equal(t, `

Could you review the launch plan, ? Thanks

`, got) + + got, err = withoutMention("

Hi " + fakebasecamp.Mention(fakebasecamp.AgentID) + "

") + require.NoError(t, err) + assert.Equal(t, "

Hi

", got) + + for _, content := range []string{ + `

No mention here

`, + `

` + fakebasecamp.Mention(1) + fakebasecamp.Mention(2) + `

`, + `

`, + } { + _, err := withoutMention(content) + assert.Error(t, err, "%q", content) + } +} + +func validDevEnv() map[string]string { + return map[string]string{ + envBaseURL: "http://3.basecamp.localhost:3001", + envAgentProfile: "agent", + envOperatorProfile: "rob", + envUntrustedProfile: "jason", + envProjectID: "2085958499", + } +} + +func envOf(m map[string]string) func(string) string { + return func(key string) string { return m[key] } +} diff --git a/e2e/connect/failures_test.go b/e2e/connect/failures_test.go index d5678f23c..722a9c3c8 100644 --- a/e2e/connect/failures_test.go +++ b/e2e/connect/failures_test.go @@ -37,15 +37,16 @@ const MsgShuttingDown = "connector: shutting down" // not have. func TestSIGINTShutsTheConnectorDownCleanly(t *testing.T) { t.Parallel() - _, c := streaming(t) - out := c.Stop(t) - - shutdown := out.Logged(MsgShuttingDown) - require.Len(t, shutdown, 1, out.Transcript()) - assert.Equal(t, "interrupt", shutdown[0].Attrs["signal"]) - for _, l := range out.Logs { - assert.NotEqual(t, "ERROR", l.Level, "%q", l.Raw) - } + onEachTarget(t, func(t *testing.T, tg Target) { + out := streamingOn(t, tg).Stop(t) + + shutdown := out.Logged(MsgShuttingDown) + require.Len(t, shutdown, 1, out.Transcript()) + assert.Equal(t, "interrupt", shutdown[0].Attrs["signal"]) + for _, l := range out.Logs { + assert.NotEqual(t, "ERROR", l.Level, "%q", l.Raw) + } + }) } // A second connector for an agent already running, started from another @@ -53,19 +54,28 @@ func TestSIGINTShutsTheConnectorDownCleanly(t *testing.T) { // holds the agent, and asks Basecamp for nothing, not even a token. func TestASecondConnectorForTheSameAgentRefusesAtOnce(t *testing.T) { t.Parallel() - h, first := streaming(t) - before := len(h.Fake.Requests()) + onEachTarget(t, func(t *testing.T, tg Target) { + first := streamingOn(t, tg) + fake, observed := fakeOf(tg) + before := 0 + if observed { + before = len(fake.Requests()) + } - second := h.Start(t, t.TempDir(), "connect", "-P", Agent).Wait(t) - require.Equal(t, exitLock, second.ExitCode, second.Transcript()) - assert.Empty(t, second.Lines, "nothing on stdout") - refusal := strings.Join(rawLogs(second), "\n") - assert.Contains(t, refusal, strconv.Itoa(first.Pid()), "the refusal names the process that holds the agent") + second := tg.CLI().Start(t, t.TempDir(), "connect", "-P", tg.Agent().Profile).Wait(t) + require.Equal(t, exitLock, second.ExitCode, second.Transcript()) + assert.Empty(t, second.Lines, "nothing on stdout") + refusal := strings.Join(rawLogs(second), "\n") + assert.Contains(t, refusal, strconv.Itoa(first.Pid()), "the refusal names the process that holds the agent") - for _, r := range h.Fake.Requests()[before:] { - t.Errorf("the refused connector reached the fake: %s %s", r.Method, r.Path) - } - first.Stop(t) + // Only the fake sees every request made of it. + if observed { + for _, r := range fake.Requests()[before:] { + t.Errorf("the refused connector reached the fake: %s %s", r.Method, r.Path) + } + } + first.Stop(t) + }) } // SecondAgentID is a second agent on the default project, connected through @@ -83,6 +93,7 @@ var SecondAgent = Profile{Name: "second", ClientID: SecondAgentClientID} // off only what was asked of it. func TestTwoAgentsRunSideBySideAndEachGetsItsOwnMentions(t *testing.T) { t.Parallel() + fakeOnly(t, "it needs a second agent connected in the same world") w := world() w.People[SecondAgentID] = &fakebasecamp.Person{ID: SecondAgentID, Name: SecondAgentName, Agent: true} w.Agents[SecondAgentClientID] = &fakebasecamp.AgentClient{ @@ -132,6 +143,7 @@ const disconnected = "was disconnected in Basecamp, or connected on another comp // how to reconnect. func TestAnAgentDisconnectedWhileRunningStopsTheConnector(t *testing.T) { t.Parallel() + fakeOnly(t, "it disconnects the agent in Basecamp") h, c := streaming(t) h.Fake.Disconnect(fakebasecamp.AgentClientID) ev := comment(h, fakebasecamp.OperatorID, askAgent(), fakebasecamp.Live) @@ -145,6 +157,7 @@ func TestAnAgentDisconnectedWhileRunningStopsTheConnector(t *testing.T) { // and the reconnect's stream ticket is refused. func TestAnAgentDisconnectedWhileRunningStopsTheConnectorOnReconnect(t *testing.T) { t.Parallel() + fakeOnly(t, "it disconnects the agent in Basecamp and drops the live connection") h, c := streaming(t) h.Fake.Disconnect(fakebasecamp.AgentClientID) require.Equal(t, 1, h.Fake.DropCable()) @@ -157,6 +170,7 @@ func TestAnAgentDisconnectedWhileRunningStopsTheConnectorOnReconnect(t *testing. // renewal is refused invalid_client, and the connector exits 3, saying so. func TestAnAgentConnectedElsewhereStopsTheConnectorAtItsNextRenewal(t *testing.T) { t.Parallel() + fakeOnly(t, "it connects the agent again on another computer, and needs tokens that last a second") h := NewHarness(t, shortTokens(world())) h.Setup(t) c := h.Connect(t) @@ -182,6 +196,7 @@ func TestAnAgentConnectedElsewhereStopsTheConnectorAtItsNextRenewal(t *testing.T // comes up and hands off a mention. func TestARateLimitedTokenAtStartIsWaitedOut(t *testing.T) { t.Parallel() + fakeOnly(t, "it rate-limits the token endpoint, and needs tokens that last a second") h := NewHarness(t, shortTokens(world())) h.Setup(t) h.waitTokenDue(t, DefaultAgent) @@ -209,6 +224,7 @@ func TestARateLimitedTokenAtStartIsWaitedOut(t *testing.T) { // and it is handed off once. func TestAnEventCommittedWhileTheCableIsDownArrivesByCatchUp(t *testing.T) { t.Parallel() + fakeOnly(t, "it holds the reconnect and publishes on the poll lane alone") h, c := streaming(t) // Hold the reconnect at its stream ticket, so the event is committed // while the connector is certainly between connections. @@ -234,6 +250,7 @@ func TestAnEventCommittedWhileTheCableIsDownArrivesByCatchUp(t *testing.T) { // option shortens, so this runs once. func TestAStaleCableIsReplaced(t *testing.T) { t.Parallel() + fakeOnly(t, "it silences the live connection's pings") h, c := streaming(t) h.Fake.StopPings() c.WaitStreaming(t, 2) @@ -248,6 +265,7 @@ func TestAStaleCableIsReplaced(t *testing.T) { // verdict and handed off once. func TestAReadThatFailsTwiceThenServesIsAdmittedInOneDecision(t *testing.T) { t.Parallel() + fakeOnly(t, "it fails recording reads") h, c := streaming(t) failing := h.Fake.Inject(fakebasecamp.ServerError(2, fakebasecamp.RouteComment)) ev := comment(h, fakebasecamp.OperatorID, askAgent(), fakebasecamp.Live) @@ -274,22 +292,22 @@ func TestAReadThatFailsTwiceThenServesIsAdmittedInOneDecision(t *testing.T) { // off is handed off now, once. func TestARequestAdmittedBeforeACrashIsHandedOffOnRestart(t *testing.T) { t.Parallel() - h := NewHarness(t, world()) - h.Setup(t) - held := h.Connect(t, "--hold") - held.WaitStreaming(t, 1) - ev := comment(h, fakebasecamp.OperatorID, askAgent(), fakebasecamp.Live) - before := held.WaitFor(t, "the event line", func(o Output) bool { return len(o.EventsFor(ev.ID)) > 0 }) - require.Equal(t, "admitted", before.EventsFor(ev.ID)[0].State) - require.Empty(t, before.RequestsFor(ev.ID), "held: nothing is handed off") - held.Kill(t) - require.Equal(t, -1, held.Wait(t).ExitCode, "killed") - - h.MustRun(t, "connect", "release", "-P", Agent) - c := h.Connect(t) - c.WaitFor(t, "the request line", func(o Output) bool { return len(o.RequestsFor(ev.ID)) > 0 }) - out := c.Stop(t) - assert.Len(t, out.RequestsFor(ev.ID), 1) + onEachTarget(t, func(t *testing.T, tg Target) { + held := connectAs(t, tg, "--hold") + held.WaitStreaming(t, 1) + p := tg.OperatorMentionsAgent(t) + before := held.WaitFor(t, "the event line", func(o Output) bool { return len(o.EventsOn(p)) > 0 }) + require.Equal(t, "admitted", before.EventsOn(p)[0].State) + require.Empty(t, before.RequestsOn(p), "held: nothing is handed off") + held.Kill(t) + require.Equal(t, -1, held.Wait(t).ExitCode, "killed") + + tg.CLI().MustRun(t, "connect", "release", "-P", tg.Agent().Profile) + c := connectAs(t, tg) + c.WaitFor(t, "the request line", func(o Output) bool { return len(o.RequestsOn(p)) > 0 }) + out := c.Stop(t) + assert.Len(t, out.RequestsOn(p), 1) + }) } // Admission falls behind the feed: Basecamp holds every recording read @@ -298,6 +316,7 @@ func TestARequestAdmittedBeforeACrashIsHandedOffOnRestart(t *testing.T) { // the backlog falls below it. func TestABacklogPastItsWarningDepthIsReportedAndRecovers(t *testing.T) { t.Parallel() + fakeOnly(t, "it holds every recording read open") h, c := streaming(t) reads := h.Fake.Gate(fakebasecamp.RecordingReads...) // Each admission worker takes one event off the backlog and is held at diff --git a/e2e/connect/fake_target_test.go b/e2e/connect/fake_target_test.go new file mode 100644 index 000000000..312d49ec1 --- /dev/null +++ b/e2e/connect/fake_target_test.go @@ -0,0 +1,67 @@ +//go:build linux || darwin + +package connect + +import ( + "strconv" + "strings" + "testing" + + "github.com/basecamp/basecamp-cli/internal/connector/fakebasecamp" +) + +// fakeTarget is the fake Basecamp as a Target: the default world with a +// bystander on the served project, in a home of the test's own, with the +// agent connected and set up through the CLI. A comment is put in the world +// and its event published live, as Basecamp publishes it first. +type fakeTarget struct { + h *Harness +} + +func newFakeTarget(t *testing.T) *fakeTarget { + t.Helper() + h := NewHarness(t, world()) + h.Setup(t) + return &fakeTarget{h: h} +} + +func (f *fakeTarget) CLI() *Harness { return f.h } + +func (f *fakeTarget) Agent() ServingAgent { + return ServingAgent{Profile: Agent, PersonID: fakebasecamp.AgentID, AccountID: fakebasecamp.AccountID} +} + +func (f *fakeTarget) OperatorMentionsAgent(t *testing.T) Posted { + t.Helper() + return f.post(fakebasecamp.OperatorID, fakebasecamp.OperatorName) +} + +func (f *fakeTarget) UntrustedMentionsAgent(t *testing.T) Posted { + t.Helper() + return f.post(BystanderID, BystanderName) +} + +func (f *fakeTarget) post(author int64, name string) Posted { + ev := comment(f.h, author, askAgent(), fakebasecamp.Live) + return Posted{ + RecordingID: ev.RecordingID, + ParentID: kickoffID, + BucketID: fakebasecamp.ProjectID, + ProjectName: fakebasecamp.ProjectName, + Type: "Comment", + URL: f.h.Fake.URL() + "/" + strconv.FormatInt(fakebasecamp.AccountID, 10) + "/comments/" + strconv.FormatInt(ev.RecordingID, 10), + AuthorID: author, + AuthorName: name, + Instruction: strings.Replace(askAgent(), fakebasecamp.Mention(fakebasecamp.AgentID), " ", 1), + } +} + +// fakeOf is tg's fake Basecamp, for an assertion only the fake can make +// (every request it answered); false against a real Basecamp. +func fakeOf(tg Target) (*fakebasecamp.Server, bool) { + f, ok := tg.(*fakeTarget) + if !ok { + return nil, false + } + return f.h.Fake, true +} diff --git a/e2e/connect/harness_test.go b/e2e/connect/harness_test.go index df70e0e5e..e40ff31c3 100644 --- a/e2e/connect/harness_test.go +++ b/e2e/connect/harness_test.go @@ -40,15 +40,23 @@ type Profile struct { // DefaultAgent is the default world's agent, as the Agent profile. var DefaultAgent = Profile{Name: Agent, ClientID: fakebasecamp.AgentClientID} -// Harness is one test's Basecamp, and a home of its own to run the CLI in: +// Harness runs the CLI for one test: the environment every command gets, +// and the directory it runs in. +// +// Against the fake it is also the test's Basecamp, and a home of its own: // its config, its credentials and the connector's state are the test's -// alone. +// alone. Against a local Basecamp (devTarget) it has no fake and no home of +// its own: the developer's config and credentials are used as they are, and +// only the connector's state is the test's. type Harness struct { t *testing.T - // Fake is the Basecamp the CLI talks to. + // Fake is the Basecamp the CLI talks to; nil against a local Basecamp. Fake *fakebasecamp.Server - // Home is HOME; the XDG directories are under it. + // Home is HOME against the fake; the XDG directories are under it. + // Empty against a local Basecamp, whose HOME is the developer's. Home string + // Dir is the directory every command runs in. + Dir string env []string } @@ -59,7 +67,7 @@ func NewHarness(t *testing.T, world *fakebasecamp.World, opts ...fakebasecamp.Op t.Helper() s := fakebasecamp.Start(t, world, opts...) home := t.TempDir() - return &Harness{t: t, Fake: s, Home: home, env: cliEnv(home, s.URL())} + return &Harness{t: t, Fake: s, Home: home, Dir: home, env: cliEnv(home, s.URL())} } // cliEnv is the whole environment a CLI subprocess gets. It starts empty @@ -98,13 +106,20 @@ type Result struct { // how it ended. It fails the test only when the command could not be run or // did not finish. func (h *Harness) Run(t *testing.T, args ...string) Result { + t.Helper() + return h.run(t.Context(), t, args...) +} + +// run is Run under ctx: t.Context() for a command the test runs, and a +// context of its own for one a cleanup runs, after t.Context() has ended. +func (h *Harness) run(parent context.Context, t *testing.T, args ...string) Result { t.Helper() bin := binary(t) // before the clock starts: the first build can be slow - ctx, cancel := context.WithTimeout(t.Context(), waitFor) + ctx, cancel := context.WithTimeout(parent, waitFor) defer cancel() cmd := exec.CommandContext(ctx, bin, args...) cmd.Env = h.env - cmd.Dir = h.Home + cmd.Dir = h.Dir var stdout, stderr bytes.Buffer cmd.Stdout, cmd.Stderr = &stdout, &stderr err := cmd.Run() @@ -157,7 +172,7 @@ func (h *Harness) SetupProfile(t *testing.T, p Profile) { func (h *Harness) AnotherComputer(t *testing.T) *Harness { t.Helper() home := t.TempDir() - return &Harness{t: t, Fake: h.Fake, Home: home, env: cliEnv(home, h.Fake.URL())} + return &Harness{t: t, Fake: h.Fake, Home: home, Dir: home, env: cliEnv(home, h.Fake.URL())} } // ConnectJSON is where `connect setup` writes the Agent profile's diff --git a/e2e/connect/main_test.go b/e2e/connect/main_test.go index 881cbb1f3..6fc1b0715 100644 --- a/e2e/connect/main_test.go +++ b/e2e/connect/main_test.go @@ -1,11 +1,12 @@ //go:build linux || darwin // Package connect runs `basecamp connect` end to end: the real binary, as -// subprocesses, against the fake Basecamp in internal/connector/fakebasecamp. +// subprocesses, against the fake Basecamp in internal/connector/fakebasecamp, +// or, under `make test-connect-dev`, against a local Basecamp (see Target). // -// Every test sets the agent up the way a person does, with `basecamp auth -// agent connect` and `basecamp connect setup`, in a home of its own, and -// then runs `basecamp connect` and reads what it writes. Its stdout is read +// Against the fake, every test sets the agent up the way a person does, with +// `basecamp auth agent connect` and `basecamp connect setup`, in a home of +// its own, and then runs `basecamp connect` and reads what it writes. Its stdout is read // as the protocol it is (typed NDJSON lines), its stderr as the slog lines // they are, both continuously, so the process never blocks on a full pipe. // Nothing waits on a timer: a test waits for a line, or for the fake to have diff --git a/e2e/connect/target_test.go b/e2e/connect/target_test.go new file mode 100644 index 000000000..d7bc0aee2 --- /dev/null +++ b/e2e/connect/target_test.go @@ -0,0 +1,152 @@ +//go:build linux || darwin + +package connect + +import "testing" + +// Target is the Basecamp a scenario runs against, in the words the scenario +// uses: who serves what, and what people do there. A scenario written +// against it runs on either target unchanged, and passing on the fake and on +// a local Basecamp is what holds the fake to Basecamp's behavior. +// +// There are two. fakeTarget serves a world of its own on the fake, set up +// the way a person sets an agent up. devTarget drives a local Basecamp with +// the real CLI and the developer's own profiles; it runs only under `make +// test-connect-dev`. Scenarios that need a fault only the fake can produce +// (a dropped cable, a rate-limited token, a disconnected agent) are written +// against the fake directly, and say so with fakeOnly. +type Target interface { + // CLI runs the CLI against the target, as the developer would: its + // environment points the CLI at this Basecamp, and the connector's state + // is the test's alone. + CLI() *Harness + // Agent is the agent set up to serve the project, as its profile. + Agent() ServingAgent + // OperatorMentionsAgent has the operator, whom the agent takes + // instructions from, post a comment that mentions the agent and asks + // it for something, on a recording in the served project. It returns + // the comment as Basecamp holds it. + OperatorMentionsAgent(t *testing.T) Posted + // UntrustedMentionsAgent has someone on the served project whom the + // agent takes no instructions from post the same. + UntrustedMentionsAgent(t *testing.T) Posted +} + +// ServingAgent is the agent a target set up, and where. +type ServingAgent struct { + // Profile is the CLI profile that holds the agent's credential. + Profile string + // PersonID is the agent's Person id. + PersonID int64 + // AccountID is the account it serves in. + AccountID int64 +} + +// Posted is a comment someone posted, as Basecamp holds it: what a request +// line for it says, field by field. +type Posted struct { + // RecordingID is the comment's id. The comment.created event for it + // carries it as its recording_id. + RecordingID int64 + // ParentID is the recording the comment is on, where a reply goes. + ParentID int64 + // BucketID and ProjectName are the served project's. + BucketID int64 + ProjectName string + // Type, Title and URL are the comment's own. + Type string + Title string + URL string + // AuthorID and AuthorName are whoever posted it. + AuthorID int64 + AuthorName string + // Instruction is the comment's content with the agent's mention taken + // out, leaving a space where it was: what a request line hands off. + Instruction string +} + +// eventType is the event a posted comment makes. +const eventType = "comment.created" + +// onEachTarget runs scenario against the fake, as part of every `go test`, +// or against the local Basecamp instead when `make test-connect-dev` asks +// for it. Each target is a subtest named for it, so a failure says which +// Basecamp behaved differently. +func onEachTarget(t *testing.T, scenario func(t *testing.T, tg Target)) { + t.Helper() + if !devRequested() { + t.Run("fake", func(t *testing.T) { + t.Parallel() + scenario(t, newFakeTarget(t)) + }) + return + } + t.Run("dev", func(t *testing.T) { + // One scenario at a time against a local Basecamp: they share its + // agent, its credential and its project. + devSerial.Lock() + defer devSerial.Unlock() + scenario(t, newDevTarget(t)) + }) +} + +// connectAs starts `basecamp connect` for tg's agent with extra flags. +func connectAs(t *testing.T, tg Target, extra ...string) *Connector { + t.Helper() + h := tg.CLI() + return h.Start(t, h.Dir, append([]string{"connect", "-P", tg.Agent().Profile}, extra...)...) +} + +// streamingOn starts the connector for tg's agent and waits until an event +// published live reaches it. +func streamingOn(t *testing.T, tg Target) *Connector { + t.Helper() + c := connectAs(t, tg) + c.WaitStreaming(t, 1) + return c +} + +// PointersOn are the pointer lines for the event p made. +func (o Output) PointersOn(p Posted) []PointerLine { + var out []PointerLine + for _, l := range o.Pointers() { + if l.RecordingID == p.RecordingID && l.EventType == eventType { + out = append(out, l) + } + } + return out +} + +// EventsOn are the verdict lines for the event p made. +func (o Output) EventsOn(p Posted) []EventLine { + var out []EventLine + for _, l := range o.Events() { + if l.RecordingID == p.RecordingID && l.EventType == eventType { + out = append(out, l) + } + } + return out +} + +// RequestsOn are the request lines for the event p made. +func (o Output) RequestsOn(p Posted) []RequestLine { + var out []RequestLine + for _, l := range o.Requests() { + if l.Recording.RecordingID == p.RecordingID && l.EventType == eventType { + out = append(out, l) + } + } + return out +} + +// RequestsAbout are the request lines for anything on the recording p is +// on: p itself, the recording, or another comment on it. +func (o Output) RequestsAbout(p Posted) []RequestLine { + var out []RequestLine + for _, l := range o.Requests() { + if l.Recording.RecordingID == p.RecordingID || l.Recording.RecordingID == p.ParentID || l.ReplyTo.RecordingID == p.ParentID { + out = append(out, l) + } + } + return out +}