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 +}