Skip to content

Commit dcc6493

Browse files
cmacknzcmacknz
authored
Add missing contexts (and therefore timeouts) to integration test code (#3892)
* Add missing IsPolicyRevision context. * Add missing context to Fleet tools API. * Add context to ConnectedToFleet method. * Add missing context to ES methods. * Add missing context to serverless delete deployment * Add missing serverless provisioning contexts. * Add missing context in magefile. * Use separate context in t.Cleanup calls. * Add missing countAPMTraces context. Propagate context through the state machine next, After, and Before methods. * Use minutes instead of seconds in serverless tests. * Fix memory aliasing warnings in machine.go.
1 parent 52a4275 commit dcc6493

19 files changed

+146
-120
lines changed

‎magefile.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1887,7 +1887,9 @@ func createTestRunner(matrix bool, singleTest string, goTestFlags string, batche
18871887
}
18881888

18891889
} else if stackProvisionerMode == ess.ProvisionerServerless {
1890-
stackProvisioner, err = ess.NewServerlessProvisioner(provisionCfg)
1890+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
1891+
defer cancel()
1892+
stackProvisioner, err = ess.NewServerlessProvisioner(ctx, provisionCfg)
18911893
if err != nil {
18921894
return nil, err
18931895
}

‎pkg/testing/ess/serverless.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,9 @@ func (srv *ServerlessClient) DeploymentIsReady(ctx context.Context) (bool, error
139139
}
140140

141141
// DeleteDeployment deletes the deployment
142-
func (srv *ServerlessClient) DeleteDeployment() error {
142+
func (srv *ServerlessClient) DeleteDeployment(ctx context.Context) error {
143143
endpoint := fmt.Sprintf("%s/api/v1/serverless/projects/%s/%s", serverlessURL, srv.proj.Type, srv.proj.ID)
144-
req, err := http.NewRequestWithContext(context.Background(), "DELETE", endpoint, nil)
144+
req, err := http.NewRequestWithContext(ctx, "DELETE", endpoint, nil)
145145
if err != nil {
146146
return fmt.Errorf("error creating HTTP request: %w", err)
147147
}

‎pkg/testing/ess/serverless_provisioner.go

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,12 +47,12 @@ type ServerlessRegions struct {
4747
}
4848

4949
// NewServerlessProvisioner creates a new StackProvisioner instance for serverless
50-
func NewServerlessProvisioner(cfg ProvisionerConfig) (runner.StackProvisioner, error) {
50+
func NewServerlessProvisioner(ctx context.Context, cfg ProvisionerConfig) (runner.StackProvisioner, error) {
5151
prov := &ServerlessProvisioner{
5252
cfg: cfg,
5353
log: &defaultLogger{wrapped: logp.L()},
5454
}
55-
err := prov.CheckCloudRegion()
55+
err := prov.CheckCloudRegion(ctx)
5656
if err != nil {
5757
return nil, fmt.Errorf("error checking region setting: %w", err)
5858
}
@@ -178,7 +178,7 @@ func (prov *ServerlessProvisioner) Delete(ctx context.Context, stack runner.Stac
178178
client.proj.Credentials.Password = stack.Password
179179

180180
prov.log.Logf("Destroying serverless stack %s [stack_id: %s, deployment_id: %s]", stack.Version, stack.ID, deploymentID)
181-
err = client.DeleteDeployment()
181+
err = client.DeleteDeployment(ctx)
182182
if err != nil {
183183
return fmt.Errorf("error removing serverless stack %s [stack_id: %s, deployment_id: %s]: %w", stack.Version, stack.ID, deploymentID, err)
184184
}
@@ -188,10 +188,10 @@ func (prov *ServerlessProvisioner) Delete(ctx context.Context, stack runner.Stac
188188
// CheckCloudRegion checks to see if the provided region is valid for the serverless
189189
// if we have an invalid region, overwrite with a valid one.
190190
// The "normal" and serverless ESS APIs have different regions, hence why we need this.
191-
func (prov *ServerlessProvisioner) CheckCloudRegion() error {
191+
func (prov *ServerlessProvisioner) CheckCloudRegion(ctx context.Context) error {
192192
urlPath := fmt.Sprintf("%s/api/v1/serverless/regions", serverlessURL)
193193

194-
httpHandler, err := http.NewRequestWithContext(context.Background(), "GET", urlPath, nil)
194+
httpHandler, err := http.NewRequestWithContext(ctx, "GET", urlPath, nil)
195195
if err != nil {
196196
return fmt.Errorf("error creating new httpRequest: %w", err)
197197
}

‎pkg/testing/ess/serverless_test.go

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@ import (
1616
)
1717

1818
func TestProvisionGetRegions(t *testing.T) {
19+
ctx, cancel := context.WithTimeout(context.Background(), 4*time.Minute)
20+
defer cancel()
21+
1922
_ = logp.DevelopmentSetup()
2023
key, found, err := GetESSAPIKey()
2124
if !found {
@@ -29,13 +32,16 @@ func TestProvisionGetRegions(t *testing.T) {
2932
cfg: cfg,
3033
log: &defaultLogger{wrapped: logp.L()},
3134
}
32-
err = prov.CheckCloudRegion()
35+
err = prov.CheckCloudRegion(ctx)
3336
require.NoError(t, err)
3437
require.NotEqual(t, "bad-region-ID", prov.cfg.Region)
3538

3639
}
3740

3841
func TestStackProvisioner(t *testing.T) {
42+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
43+
defer cancel()
44+
3945
_ = logp.DevelopmentSetup()
4046
key, found, err := GetESSAPIKey()
4147
if !found {
@@ -45,12 +51,10 @@ func TestStackProvisioner(t *testing.T) {
4551
require.True(t, found)
4652

4753
cfg := ProvisionerConfig{Region: "aws-eu-west-1", APIKey: key}
48-
provClient, err := NewServerlessProvisioner(cfg)
54+
provClient, err := NewServerlessProvisioner(ctx, cfg)
4955
require.NoError(t, err)
5056
request := runner.StackRequest{ID: "stack-test-one", Version: "8.9.0"}
5157

52-
ctx, cancel := context.WithTimeout(context.Background(), time.Minute*5)
53-
defer cancel()
5458
stack, err := provClient.Create(ctx, request)
5559
require.NoError(t, err)
5660
t.Logf("got results:")
@@ -78,7 +82,7 @@ func TestStartServerless(t *testing.T) {
7882
key,
7983
&defaultLogger{wrapped: logp.L()})
8084

81-
ctx, cancel := context.WithTimeout(context.Background(), time.Second*240)
85+
ctx, cancel := context.WithTimeout(context.Background(), 4*time.Minute)
8286
defer cancel()
8387

8488
req := ServerlessRequest{Name: "ingest-e2e-test", RegionID: "aws-eu-west-1"}
@@ -95,6 +99,6 @@ func TestStartServerless(t *testing.T) {
9599
t.Logf("got endpoints: %#v", clientHandle.proj.Endpoints)
96100
t.Logf("got auth: %#v", clientHandle.proj.Credentials)
97101

98-
err = clientHandle.DeleteDeployment()
102+
err = clientHandle.DeleteDeployment(ctx)
99103
require.NoError(t, err)
100104
}

‎pkg/testing/fixture.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,7 @@ func (f *Fixture) Run(ctx context.Context, states ...State) error {
480480
}
481481
case state := <-stateCh:
482482
if smInstance != nil {
483-
cfg, cont, err := smInstance.next(state)
483+
cfg, cont, err := smInstance.next(ctx, state)
484484
if err != nil {
485485
killProc()
486486
return fmt.Errorf("state management failed with unexpected error: %w", err)

‎pkg/testing/machine.go

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package testing
66

77
import (
8+
"context"
89
"errors"
910
"fmt"
1011

@@ -153,10 +154,12 @@ type State struct {
153154

154155
// Before is called once when this state is the next state trying to be resolved.
155156
// This is called before the configuration is sent to the Elastic Agent if `Configuration` is set.
156-
Before func() error
157+
// Before is passed the context that was passed to fixture.Run().
158+
Before func(ctx context.Context) error
157159

158160
// After is called once after this state has been resolved and the next state is going to be tried.
159-
After func() error
161+
// After is passed the context that was passed to fixture.Run().
162+
After func(ctx context.Context) error
160163
}
161164

162165
// Validate ensures correctness of state definition.
@@ -208,7 +211,7 @@ func newStateMachine(states []State) (*stateMachine, error) {
208211
}, nil
209212
}
210213

211-
func (sm *stateMachine) next(agentState *client.AgentState) (string, bool, error) {
214+
func (sm *stateMachine) next(ctx context.Context, agentState *client.AgentState) (string, bool, error) {
212215
if sm.current >= len(sm.states) {
213216
// already made it to the end, should be stopped
214217
return "", false, nil
@@ -224,7 +227,7 @@ func (sm *stateMachine) next(agentState *client.AgentState) (string, bool, error
224227
}
225228
if reached {
226229
if state.After != nil {
227-
if err := state.After(); err != nil {
230+
if err := state.After(ctx); err != nil {
228231
return "", false, fmt.Errorf("failed to perform After on state %d: %w", sm.current, err)
229232
}
230233
}
@@ -235,7 +238,7 @@ func (sm *stateMachine) next(agentState *client.AgentState) (string, bool, error
235238
}
236239
next := sm.states[sm.current]
237240
if next.Before != nil {
238-
if err := next.Before(); err != nil {
241+
if err := next.Before(ctx); err != nil {
239242
return "", false, fmt.Errorf("failed to perform Before on state %d: %w", sm.current, err)
240243
}
241244
}
@@ -244,7 +247,7 @@ func (sm *stateMachine) next(agentState *client.AgentState) (string, bool, error
244247
return next.Configure, true, nil
245248
}
246249
// no configuration on this state; so we can determine if this next state has already been reached as well
247-
return sm.next(agentState)
250+
return sm.next(ctx, agentState)
248251
}
249252
return "", true, nil
250253
}
@@ -283,7 +286,9 @@ func stateComponentsReached(components map[string]ComponentState, agentComponent
283286
return false
284287
}
285288
found := make(map[string]bool)
286-
for _, agentComp := range agentComponents {
289+
for i := range agentComponents {
290+
// Index the array to avoid aliasing a temporary loop value when taking the address below.
291+
agentComp := agentComponents[i]
287292
state, ok := components[agentComp.ID]
288293
if !ok {
289294
if strict {
@@ -298,7 +303,7 @@ func stateComponentsReached(components map[string]ComponentState, agentComponent
298303
return false
299304
}
300305
}
301-
for compID, _ := range components {
306+
for compID := range components {
302307
_, ok := found[compID]
303308
if !ok {
304309
// was not found
@@ -334,7 +339,9 @@ func stateComponentUnitsReached(units map[ComponentUnitKey]ComponentUnitState, c
334339
return false
335340
}
336341
found := make(map[ComponentUnitKey]bool)
337-
for _, compUnit := range compUnits {
342+
for i := range compUnits {
343+
// Index the array to avoid aliasing a temporary loop value when taking the address below.
344+
compUnit := compUnits[i]
338345
key := ComponentUnitKey{UnitType: compUnit.UnitType, UnitID: compUnit.UnitID}
339346
state, ok := units[key]
340347
if !ok {
@@ -350,7 +357,7 @@ func stateComponentUnitsReached(units map[ComponentUnitKey]ComponentUnitState, c
350357
return false
351358
}
352359
}
353-
for key, _ := range units {
360+
for key := range units {
354361
_, ok := found[key]
355362
if !ok {
356363
// was not found

‎pkg/testing/machine_test.go

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
package testing
66

77
import (
8+
"context"
89
"errors"
910
"testing"
1011

@@ -789,6 +790,9 @@ func TestStateMachine(t *testing.T) {
789790

790791
for _, scenario := range scenarios {
791792
t.Run(scenario.Name, func(t *testing.T) {
793+
ctx, cancel := context.WithCancel(context.Background())
794+
defer cancel()
795+
792796
m, err := newStateMachine(scenario.States)
793797
if scenario.Err != nil {
794798
require.Error(t, err)
@@ -797,7 +801,7 @@ func TestStateMachine(t *testing.T) {
797801
require.NoError(t, err)
798802
}
799803
for _, nextCall := range scenario.AgentStates {
800-
cfg, cont, err := m.next(nextCall.AgentState)
804+
cfg, cont, err := m.next(ctx, nextCall.AgentState)
801805
if nextCall.Err != nil {
802806
require.Error(t, err)
803807
require.Equal(t, nextCall.Err.Error(), err.Error())
@@ -821,32 +825,35 @@ func TestStateMachine_Before_After(t *testing.T) {
821825
{
822826
Configure: "my config",
823827
AgentState: NewClientState(client.Configuring),
824-
Before: func() error {
828+
Before: func(ctx context.Context) error {
825829
firstBefore = true
826830
return nil
827831
},
828-
After: func() error {
832+
After: func(ctx context.Context) error {
829833
firstAfter = true
830834
return nil
831835
},
832836
},
833837
{
834838
AgentState: NewClientState(client.Healthy),
835-
Before: func() error {
839+
Before: func(ctx context.Context) error {
836840
secondBefore = true
837841
return nil
838842
},
839-
After: func() error {
843+
After: func(ctx context.Context) error {
840844
secondAfter = true
841845
return nil
842846
},
843847
},
844848
}
845849

850+
ctx, cancel := context.WithCancel(context.Background())
851+
defer cancel()
852+
846853
m, err := newStateMachine(states)
847854
require.NoError(t, err)
848855

849-
cfg, cont, err := m.next(&client.AgentState{
856+
cfg, cont, err := m.next(ctx, &client.AgentState{
850857
State: client.Configuring,
851858
})
852859
require.NoError(t, err)
@@ -855,7 +862,7 @@ func TestStateMachine_Before_After(t *testing.T) {
855862
require.True(t, firstBefore)
856863
require.False(t, firstAfter)
857864

858-
cfg, cont, err = m.next(&client.AgentState{
865+
cfg, cont, err = m.next(ctx, &client.AgentState{
859866
State: client.Configuring,
860867
})
861868
require.NoError(t, err)
@@ -865,7 +872,7 @@ func TestStateMachine_Before_After(t *testing.T) {
865872
require.True(t, secondBefore)
866873
require.False(t, secondAfter)
867874

868-
cfg, cont, err = m.next(&client.AgentState{
875+
cfg, cont, err = m.next(ctx, &client.AgentState{
869876
State: client.Healthy,
870877
})
871878
require.NoError(t, err)

‎pkg/testing/tools/check/check.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,13 @@ import (
2020
// ConnectedToFleet checks if the agent defined in the fixture is connected to
2121
// Fleet Server. It uses assert.Eventually and if it fails the last error will
2222
// be printed. It returns if the agent is connected to Fleet Server or not.
23-
func ConnectedToFleet(t *testing.T, fixture *integrationtest.Fixture, timeout time.Duration) bool {
23+
func ConnectedToFleet(ctx context.Context, t *testing.T, fixture *integrationtest.Fixture, timeout time.Duration) bool {
2424
t.Helper()
2525

2626
var err error
2727
var agentStatus integrationtest.AgentStatusOutput
2828
assertFn := func() bool {
29-
agentStatus, err = fixture.ExecStatus(context.Background())
29+
agentStatus, err = fixture.ExecStatus(ctx)
3030
return agentStatus.FleetState == int(cproto.State_HEALTHY)
3131
}
3232

@@ -45,12 +45,13 @@ func ConnectedToFleet(t *testing.T, fixture *integrationtest.Fixture, timeout ti
4545
// FleetAgentStatus returns a niladic function that returns true if the agent
4646
// has reached expectedStatus; false otherwise. The returned function is intended
4747
// for use with assert.Eventually or require.Eventually.
48-
func FleetAgentStatus(t *testing.T,
48+
func FleetAgentStatus(ctx context.Context,
49+
t *testing.T,
4950
client *kibana.Client,
5051
policyID,
5152
expectedStatus string) func() bool {
5253
return func() bool {
53-
currentStatus, err := fleettools.GetAgentStatus(client, policyID)
54+
currentStatus, err := fleettools.GetAgentStatus(ctx, client, policyID)
5455
if err != nil {
5556
t.Errorf("unable to determine agent status: %s", err.Error())
5657
return false

0 commit comments

Comments
 (0)