Fix an error where exit status wasn't getting captured by the cleanup process

[moskyb]

tl;dr before:

defer cancel()
defer r.cleanup(cctx, &wg, exit)

after:

defer func(ctx context.Context, wg *sync.WaitGroup) {
	cancel()
	r.cleanup(ctx, wg, exit)
}(ctx, &wg)

previously we were capturing the value of exit at the time of the call to defer, now we're capturing the value at the time the defer is actually executed, which is what we wanted in the first place.

this PR also adds a regression test to ensure that this doesn't happen again

[DrJosh9000]

Looking good - I have one or two tweaks

[DrJosh9000]

Suggested change

	err := json.NewDecoder(bytes.NewBuffer(b)).Decode(&job)
	err := json.Unmarshal(b, &job)

[moskyb]

oh, duh

[DrJosh9000]

What's this change for?

[triarius]

I'm curious about this too. Is it because you need a json error message? I think you should craft one if so.

[moskyb]

this is a relic from when i was bughunting and flailing a bit; i think my logic was that if an endpoint that we haven't accounted for is hit it's a Very Bad Thing, and i don't think these errors are super well surfaced in the tests???? maybe? but also this doesn't really need to be here, i might just yeet it

[triarius]

I think it's better to return a 4xx error in such circumstances and raise an error from the Test method. It will be easier to debug how the route was called then.

[triarius]

I guess the error handling could happen entirely in the non-test code. Hopefully, writing code that ignores errors is something we have gotten out of the habit of doing.

[triarius]

Suggested change

	func (tae *testAgentEndpoint) finishesFor(t *testing.T, jobID string) []api.Job {
	func (tae *testAgentEndpoint) finishesFor(t *testing.T, jobID string) []api.Job {
	t.Helper()

so that if an error occurs in this method, the output will show a line number from the test method.