Skip to content

Commit

Permalink
Merge pull request #241 from buildkite/tet-472/add-retry-count
Browse files Browse the repository at this point in the history
Add retry count
  • Loading branch information
niceking authored Dec 17, 2024
2 parents 7ea9ac6 + a0b473a commit 9257f47
Show file tree
Hide file tree
Showing 9 changed files with 26 additions and 8 deletions.
4 changes: 2 additions & 2 deletions internal/api/fetch_test_plan.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import (

// FetchTestPlan fetchs a test plan from the server.
// ErrRetryTimeout is returned if the client failed to communicate with the server after exceeding the retry limit.
func (c Client) FetchTestPlan(ctx context.Context, suiteSlug string, identifier string) (*plan.TestPlan, error) {
url := fmt.Sprintf("%s/v2/analytics/organizations/%s/suites/%s/test_plan?identifier=%s", c.ServerBaseUrl, c.OrganizationSlug, suiteSlug, identifier)
func (c Client) FetchTestPlan(ctx context.Context, suiteSlug string, identifier string, jobRetryCount int) (*plan.TestPlan, error) {
url := fmt.Sprintf("%s/v2/analytics/organizations/%s/suites/%s/test_plan?identifier=%s&job_retry_count=%d", c.ServerBaseUrl, c.OrganizationSlug, suiteSlug, identifier, jobRetryCount)

var testPlan plan.TestPlan

Expand Down
12 changes: 7 additions & 5 deletions internal/api/fetch_test_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ func TestFetchTestPlan(t *testing.T) {
WithRequest("GET", "/v2/analytics/organizations/buildkite/suites/rspec/test_plan", func(b *consumer.V2RequestBuilder) {
b.Header("Authorization", matchers.Like("Bearer asdf1234"))
b.Query("identifier", matchers.Like("abc123"))
b.Query("job_retry_count", matchers.Like("0"))
}).
WillRespondWith(200, func(b *consumer.V2ResponseBuilder) {
b.Header("Content-Type", matchers.Like("application/json; charset=utf-8"))
Expand Down Expand Up @@ -62,7 +63,7 @@ func TestFetchTestPlan(t *testing.T) {

c := NewClient(cfg)

got, err := c.FetchTestPlan(context.Background(), "rspec", "abc123")
got, err := c.FetchTestPlan(context.Background(), "rspec", "abc123", 0)

if err != nil {
t.Errorf("FetchTestPlan() error = %v", err)
Expand Down Expand Up @@ -113,7 +114,8 @@ func TestFetchTestPlan_NotFound(t *testing.T) {
WithRequest("GET", "/v2/analytics/organizations/buildkite/suites/rspec/test_plan", func(b *consumer.V2RequestBuilder) {
b.
Header("Authorization", matchers.Like("Bearer asdf1234")).
Query("identifier", matchers.Like("abc123"))
Query("identifier", matchers.Like("abc123")).
Query("job_retry_count", matchers.Like("0"))
}).
WillRespondWith(404, func(b *consumer.V2ResponseBuilder) {
b.Header("Content-Type", matchers.Like("application/json; charset=utf-8"))
Expand All @@ -132,7 +134,7 @@ func TestFetchTestPlan_NotFound(t *testing.T) {

c := NewClient(cfg)

got, err := c.FetchTestPlan(context.Background(), "rspec", "abc123")
got, err := c.FetchTestPlan(context.Background(), "rspec", "abc123", 0)

if err != nil {
t.Errorf("FetchTestPlan() error = %v", err)
Expand Down Expand Up @@ -165,7 +167,7 @@ func TestFetchTestPlan_BadRequest(t *testing.T) {
}

c := NewClient(cfg)
got, err := c.FetchTestPlan(context.Background(), "my-suite", "xyz")
got, err := c.FetchTestPlan(context.Background(), "my-suite", "xyz", 0)

if requestCount > 1 {
t.Errorf("http request count = %v, want %d", requestCount, 1)
Expand Down Expand Up @@ -199,7 +201,7 @@ func TestFetchTestPlan_InternalServerError(t *testing.T) {
}

c := NewClient(cfg)
got, err := c.FetchTestPlan(context.Background(), "my-suite", "xyz")
got, err := c.FetchTestPlan(context.Background(), "my-suite", "xyz", 0)

if !errors.Is(err, ErrRetryTimeout) {
t.Errorf("FetchTestPlan() error = %v, want %v", err, ErrRetryTimeout)
Expand Down
2 changes: 2 additions & 0 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ type Config struct {
TestRunner string
// Branch is the string value of the git branch name, used by Buildkite only.
Branch string
// JobRetryCount is the count of the number of times the job has been retried.
JobRetryCount int
// errs is a map of environment variables name and the validation errors associated with them.
errs InvalidConfigError
}
Expand Down
3 changes: 3 additions & 0 deletions internal/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func setEnv(t *testing.T) {
os.Setenv("BUILDKITE_STEP_ID", "456")
os.Setenv("BUILDKITE_TEST_ENGINE_TEST_RUNNER", "rspec")
os.Setenv("BUILDKITE_TEST_ENGINE_RESULT_PATH", "tmp/rspec.json")
os.Setenv("BUILDKITE_RETRY_COUNT", "0")
}

func TestNewConfig(t *testing.T) {
Expand All @@ -44,6 +45,7 @@ func TestNewConfig(t *testing.T) {
ResultPath: "tmp/rspec.json",
SuiteSlug: "my_suite",
TestRunner: "rspec",
JobRetryCount: 0,
errs: InvalidConfigError{},
}

Expand Down Expand Up @@ -84,6 +86,7 @@ func TestNewConfig_MissingConfigWithDefault(t *testing.T) {
SuiteSlug: "my_suite",
TestRunner: "rspec",
ResultPath: "tmp/rspec.json",
JobRetryCount: 0,
}

if diff := cmp.Diff(c, want, cmpopts.IgnoreUnexported(Config{})); diff != "" {
Expand Down
1 change: 1 addition & 0 deletions internal/config/env.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ func (c Config) DumpEnv() map[string]string {
"BUILDKITE_TEST_ENGINE_TEST_RUNNER",
"BUILDKITE_STEP_ID",
"BUILDKITE_BRANCH",
"BUILDKITE_RETRY_COUNT",
}

envs := make(map[string]string)
Expand Down
7 changes: 7 additions & 0 deletions internal/config/read.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
// - BUILDKITE_TEST_ENGINE_TEST_FILE_PATTERN (TestFilePattern)
// - BUILDKITE_TEST_ENGINE_TEST_FILE_EXCLUDE_PATTERN (TestFileExcludePattern)
// - BUILDKITE_BRANCH (Branch)
// - BUILDKITE_RETRY_COUNT (JobRetryCount)
//
// If we are going to support other CI environment in the future,
// we will need to change where we read the configuration from.
Expand Down Expand Up @@ -59,6 +60,12 @@ func (c *Config) readFromEnv() error {
// used by Buildkite only, for experimental plans
c.Branch = os.Getenv("BUILDKITE_BRANCH")

JobRetryCount, err := getIntEnvWithDefault("BUILDKITE_RETRY_COUNT", 0)
c.JobRetryCount = JobRetryCount
if err != nil {
c.errs.appendFieldError("BUILDKITE_RETRY_COUNT", "was %q, must be a number", os.Getenv("BUILDKITE_RETRY_COUNT"))
}

MaxRetries, err := getIntEnvWithDefault("BUILDKITE_TEST_ENGINE_RETRY_COUNT", 0)
c.MaxRetries = MaxRetries
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions internal/config/read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func TestConfigReadFromEnv(t *testing.T) {
TestFileExcludePattern: "spec/feature/**/*_spec.rb",
TestRunner: "rspec",
ResultPath: "result.json",
JobRetryCount: 0,
}

if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion main.go
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ func fetchOrCreateTestPlan(ctx context.Context, apiClient *api.Client, cfg confi
debug.Println("Fetching test plan")

// Fetch the plan from the server's cache.
cachedPlan, err := apiClient.FetchTestPlan(ctx, cfg.SuiteSlug, cfg.Identifier)
cachedPlan, err := apiClient.FetchTestPlan(ctx, cfg.SuiteSlug, cfg.Identifier, cfg.JobRetryCount)

handleError := func(err error) (plan.TestPlan, error) {
if errors.Is(err, api.ErrRetryTimeout) {
Expand Down
2 changes: 2 additions & 0 deletions main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -798,6 +798,7 @@ func TestSendMetadata(t *testing.T) {
"BUILDKITE_TEST_ENGINE_SUITE_SLUG": "rspec",
"BUILDKITE_TEST_ENGINE_TEST_CMD": "bundle exec rspec",
"BUILDKITE_TEST_ENGINE_TEST_RUNNER": "rspec",
"BUILDKITE_RETRY_COUNT": "0",
}
for k, v := range env {
_ = os.Setenv(k, v)
Expand Down Expand Up @@ -840,6 +841,7 @@ func TestSendMetadata(t *testing.T) {
"BUILDKITE_TEST_ENGINE_TEST_FILE_PATTERN": "",
"BUILDKITE_TEST_ENGINE_TEST_RUNNER": "rspec",
"BUILDKITE_BRANCH": "",
"BUILDKITE_RETRY_COUNT": "0",
},
Statistics: runner.RunStatistics{
Total: 3,
Expand Down

0 comments on commit 9257f47

Please sign in to comment.