testutil/compose: handle docker-compose build seperately #1466
Conversation
testutil/compose/alert.go
Outdated
| for ctx.Err() == nil { | ||
| time.Sleep(time.Second * 5) | ||
| time.Sleep(time.Second * 2) |
There was a problem hiding this comment.
this will wait for 10 seconds first to let prometheus container to start and then poll alerts every 2 seconds
testutil/compose/auto.go
Outdated
| @@ -125,6 +125,11 @@ func Auto(ctx context.Context, conf AutoConfig) error { | |||
| defer cancel() | |||
| } | |||
|
|
|||
| // Build docker-compose services before executing docker-compose up. | |||
| if err = execBuild(ctx, conf.Dir); err != nil { | |||
There was a problem hiding this comment.
Found out from logs that this steps longer for the first test which is included in alerts timeout
Codecov ReportBase: 53.74% // Head: 53.97% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1466 +/- ##
==========================================
+ Coverage 53.74% 53.97% +0.23%
==========================================
Files 147 147
Lines 18684 18702 +18
==========================================
+ Hits 10042 10095 +53
+ Misses 7266 7231 -35
Partials 1376 1376
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
testutil/compose/auto.go
Outdated
| @@ -149,8 +154,7 @@ func Auto(ctx context.Context, conf AutoConfig) error { | |||
| } | |||
| } | |||
| if !alertSuccess { | |||
| log.Error(ctx, "Alerts couldn't be polled", nil) | |||
| return nil // TODO(corver): Fix this and error | |||
| return errors.New("alerts couldn't be polled, containers offline") | |||
There was a problem hiding this comment.
containers offline isn't correct. Sure it might be the most common reason, but it isn't the only reason.
testutil/compose/smoke/smoke_test.go
Outdated
| @@ -59,26 +60,30 @@ func TestSmoke(t *testing.T) { | |||
| conf.KeyGen = compose.KeyGenCreate | |||
| conf.FeatureSet = "alpha" | |||
| }, | |||
| Timeout: time.Second * 45, | |||
There was a problem hiding this comment.
suggest using a defaultTimeout variable if Timeout is empty
testutil/compose/alert.go
Outdated
| for ctx.Err() == nil { | ||
| time.Sleep(time.Second * 5) | ||
| time.Sleep(time.Second * 2) |
There was a problem hiding this comment.
rather sleep at the end of the loop (so you check for context closed directly after sleeping instead of doing logic with a possible cancelled context)
dB2510
force-pushed
the
dhruv/compose
branch
from
a5b28be
to
a5546e6
Compare
dB2510
added
the
merge when ready
Handle
docker-compose buildcommand separately before starting alerts collector. This step takes longer at first and takes almost no time to build again. This might be the reason whydefault_alphatest in smoke tests seldom completes. Also increased alert timeout forvery_largetest.category: test
ticket: #992