From f32469625fc4a8712dd661395101a482ae89a805 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 12:33:06 +0100 Subject: [PATCH 01/24] feat: add helper for reading env vars --- metrics/internal/config/config.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) create mode 100644 metrics/internal/config/config.go diff --git a/metrics/internal/config/config.go b/metrics/internal/config/config.go new file mode 100644 index 0000000..6c07cc0 --- /dev/null +++ b/metrics/internal/config/config.go @@ -0,0 +1,15 @@ +package config + +import ( + "os" + "strings" +) + +// Prefix for application specific environment variables +const envPrefix string = "RRM_METRICS" + +func FromEnv(p ...string) string { + p = append(p, envPrefix) + k := strings.Join(p, "__") + return os.Getenv(k) +} From 7249f22872a1f750d93d9ec18da33506eaa76d3d Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 12:38:36 +0100 Subject: [PATCH 02/24] feat: add JSON and Plain export encoders --- metrics/internal/export/encoder.go | 35 ++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) create mode 100644 metrics/internal/export/encoder.go diff --git a/metrics/internal/export/encoder.go b/metrics/internal/export/encoder.go new file mode 100644 index 0000000..3b3f780 --- /dev/null +++ b/metrics/internal/export/encoder.go @@ -0,0 +1,35 @@ +package export + +import ( + "encoding/json" + "fmt" + "io" +) + +// Interface for CSV, JSON and other encoders +type Encoder interface { + Encode(w io.Writer, v interface{}) error +} + +type JSONEcoder struct{} + +func (j *JSONEcoder) Encode(w io.Writer, v interface{}) error { + e := json.NewEncoder(w) + e.SetIndent("", " ") + return e.Encode(v) +} + +func NewJSONEncoder() (*JSONEcoder, error) { + return &JSONEcoder{}, nil +} + +type PlainEncoder struct{} + +func (p *PlainEncoder) Encode(w io.Writer, v interface{}) error { + _, err := fmt.Fprint(w, v) + return err +} + +func NewPlainEncoder() (*PlainEncoder, error) { + return &PlainEncoder{}, nil +} From 3c64307b24f2b5807cfdafb0cfebf6ef99f8d4ab Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 12:40:41 +0100 Subject: [PATCH 03/24] feat: add Writer and File exporters --- metrics/internal/export/exporter.go | 34 +++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 metrics/internal/export/exporter.go diff --git a/metrics/internal/export/exporter.go b/metrics/internal/export/exporter.go new file mode 100644 index 0000000..1af1449 --- /dev/null +++ b/metrics/internal/export/exporter.go @@ -0,0 +1,34 @@ +package export + +import ( + "io" + "os" +) + +type Exporter interface { + Export(v interface{}) error +} + +type WriterExporter struct { + W io.Writer + Encoder Encoder +} + +func (s *WriterExporter) Export(v interface{}) error { + return s.Encoder.Encode(s.W, v) +} + +type FileExporter struct { + Filename string + Encoder Encoder +} + +func (f *FileExporter) Export(v interface{}) error { + file, err := os.Create(f.Filename) + if err != nil { + return err + } + defer file.Close() + + return f.Encoder.Encode(file, v) +} From d8cb2e9d7c2921c0de8bd7b15490e12a02c125ff Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 12:55:48 +0100 Subject: [PATCH 04/24] feat: query GitHub GraphQL API for PRs and Releases --- metrics/go.mod | 3 + metrics/go.sum | 14 +++++ metrics/internal/github/api.go | 16 +++++ metrics/internal/github/pull_requests.go | 74 ++++++++++++++++++++++++ metrics/internal/github/releases.go | 73 +++++++++++++++++++++++ 5 files changed, 180 insertions(+) create mode 100644 metrics/internal/github/api.go create mode 100644 metrics/internal/github/pull_requests.go create mode 100644 metrics/internal/github/releases.go diff --git a/metrics/go.mod b/metrics/go.mod index c8b5d78..39c77ff 100644 --- a/metrics/go.mod +++ b/metrics/go.mod @@ -4,10 +4,13 @@ go 1.21.3 require ( github.com/google/go-cmp v0.6.0 + github.com/shurcooL/githubv4 v0.0.0-20230704064427-599ae7bbf278 github.com/spf13/cobra v1.7.0 ) require ( github.com/inconshreveable/mousetrap v1.1.0 // indirect + github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466 // indirect github.com/spf13/pflag v1.0.5 // indirect + golang.org/x/oauth2 v0.13.0 // indirect ) diff --git a/metrics/go.sum b/metrics/go.sum index 40b90be..d970134 100644 --- a/metrics/go.sum +++ b/metrics/go.sum @@ -1,12 +1,26 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= +github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= +github.com/shurcooL/githubv4 v0.0.0-20230704064427-599ae7bbf278 h1:kdEGVAV4sO46DPtb8k793jiecUEhaX9ixoIBt41HEGU= +github.com/shurcooL/githubv4 v0.0.0-20230704064427-599ae7bbf278/go.mod h1:zqMwyHmnN/eDOZOdiTohqIUKUrTFX62PNlu7IJdu0q8= +github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466 h1:17JxqqJY66GmZVHkmAsGEkcIu0oCe3AM420QDgGwZx0= +github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466/go.mod h1:9dIRpgIY7hVhoqfe0/FcYp0bpInZaT7dc3BYOprrIUE= github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I= github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +golang.org/x/net v0.16.0 h1:7eBu7KsSvFDtSXUIDbh3aqlK4DPsZ1rByC8PFfBThos= +golang.org/x/net v0.16.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= +golang.org/x/oauth2 v0.13.0 h1:jDDenyj+WgFtmV3zYVoi8aE2BwtXFLWOA67ZfNWftiY= +golang.org/x/oauth2 v0.13.0/go.mod h1:/JMhi4ZRXAf4HG9LiNmxvk+45+96RUlVThiH8FzNBn0= +google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6c= +google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= +google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= +google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/metrics/internal/github/api.go b/metrics/internal/github/api.go new file mode 100644 index 0000000..347ddc1 --- /dev/null +++ b/metrics/internal/github/api.go @@ -0,0 +1,16 @@ +package github + +import ( + "context" +) + +// Represents a GitHub repo +type Repo struct { + Owner string + Name string +} + +// GraphQLClient is satisfied by the the githubv4.Client. +type GraphQLClient interface { + Query(ctx context.Context, q interface{}, variables map[string]interface{}) error +} diff --git a/metrics/internal/github/pull_requests.go b/metrics/internal/github/pull_requests.go new file mode 100644 index 0000000..2897dda --- /dev/null +++ b/metrics/internal/github/pull_requests.go @@ -0,0 +1,74 @@ +package github + +import ( + "context" + "time" + + "github.com/shurcooL/githubv4" +) + +type PullRequest struct { + ID string + Number int + Title string + CreatedAt time.Time + UpdatedAt time.Time + ClosedAt time.Time + MergedAt time.Time +} + +// GraphQL query for GitHub Pull Requests +type PullRequestsQuery struct { + Repository struct { + Name string + Owner struct { + Login string + } + PullRequests struct { + PageInfo struct { + HasNextPage bool + EndCursor string + } + Nodes []PullRequest + } `graphql:"pullRequests(states: $states, first: $limit, after: $endCursor, orderBy: $orderBy)"` + } `graphql:"repository(owner: $owner, name: $name)"` +} + +// QueryPullRequests fetches information about merged PRs from the GitHub GraphQL API +func QueryPullRequests(gqlClient GraphQLClient, repo *Repo, limit int) ([]PullRequest, error) { + queryVariables := map[string]interface{}{ + "owner": githubv4.String(repo.Owner), + "name": githubv4.String(repo.Name), + "limit": githubv4.Int(limit), + "endCursor": (*githubv4.String)(nil), // When paginating forwards, the cursor to continue. + "states": []githubv4.PullRequestState{githubv4.PullRequestStateMerged}, + "orderBy": githubv4.IssueOrder{Field: githubv4.IssueOrderFieldUpdatedAt, Direction: githubv4.OrderDirectionDesc}, + } + + var pullRequests []PullRequest + +Loop: + for { + var query PullRequestsQuery + + err := gqlClient.Query(context.Background(), &query, queryVariables) + if err != nil { + return nil, err + } + + for _, n := range query.Repository.PullRequests.Nodes { + pullRequests = append(pullRequests, n) + if len(pullRequests) == limit { + break Loop + } + } + + if !query.Repository.PullRequests.PageInfo.HasNextPage { + break + } + + queryVariables["endCursor"] = githubv4.String(query.Repository.PullRequests.PageInfo.EndCursor) + } + + return pullRequests, nil +} diff --git a/metrics/internal/github/releases.go b/metrics/internal/github/releases.go new file mode 100644 index 0000000..03d74a8 --- /dev/null +++ b/metrics/internal/github/releases.go @@ -0,0 +1,73 @@ +package github + +import ( + "context" + "time" + + "github.com/shurcooL/githubv4" +) + +type Release struct { + Name string + TagName string + IsDraft bool + IsLatest bool + IsPrerelease bool + Description string + CreatedAt time.Time + PublishedAt time.Time +} + +type ReleasesQuery struct { + Repository struct { + Name string + Owner struct { + Login string + } + Releases struct { + PageInfo struct { + HasNextPage bool + EndCursor string + } + Nodes []Release + } `graphql:"releases(first: $limit, after: $endCursor, orderBy: $orderBy)"` + } `graphql:"repository(owner: $owner, name: $name)"` +} + +// QueryReleases fetches information about merged PRs from the GitHub GraphQL API +func QueryReleases(gqlClient GraphQLClient, repo *Repo, limit int) ([]Release, error) { + queryVariables := map[string]interface{}{ + "owner": githubv4.String(repo.Owner), + "name": githubv4.String(repo.Name), + "limit": githubv4.Int(limit), + "endCursor": (*githubv4.String)(nil), // When paginating forwards, the cursor to continue. + "orderBy": githubv4.ReleaseOrder{Field: githubv4.ReleaseOrderFieldCreatedAt, Direction: githubv4.OrderDirectionDesc}, + } + + var releases []Release + +Loop: + for { + var query ReleasesQuery + + err := gqlClient.Query(context.Background(), &query, queryVariables) + if err != nil { + return nil, err + } + + for _, n := range query.Repository.Releases.Nodes { + releases = append(releases, n) + if len(releases) == limit { + break Loop + } + } + + if !query.Repository.Releases.PageInfo.HasNextPage { + break + } + + queryVariables["endCursor"] = githubv4.String(query.Repository.Releases.PageInfo.EndCursor) + } + + return releases, nil +} From b57cf2d16a199d03c19a7bff4adf161626a3d2c5 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 13:08:13 +0100 Subject: [PATCH 05/24] feat: add factory for dependency injection --- metrics/go.mod | 6 ++- metrics/go.sum | 12 ++++++ metrics/internal/factory/factory.go | 65 +++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 metrics/internal/factory/factory.go diff --git a/metrics/go.mod b/metrics/go.mod index 39c77ff..a1665fb 100644 --- a/metrics/go.mod +++ b/metrics/go.mod @@ -6,11 +6,15 @@ require ( github.com/google/go-cmp v0.6.0 github.com/shurcooL/githubv4 v0.0.0-20230704064427-599ae7bbf278 github.com/spf13/cobra v1.7.0 + golang.org/x/oauth2 v0.13.0 ) require ( + github.com/golang/protobuf v1.5.3 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466 // indirect github.com/spf13/pflag v1.0.5 // indirect - golang.org/x/oauth2 v0.13.0 // indirect + golang.org/x/net v0.16.0 // indirect + google.golang.org/appengine v1.6.7 // indirect + google.golang.org/protobuf v1.31.0 // indirect ) diff --git a/metrics/go.sum b/metrics/go.sum index d970134..7a03d64 100644 --- a/metrics/go.sum +++ b/metrics/go.sum @@ -1,6 +1,9 @@ github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= +github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= +github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= @@ -14,12 +17,21 @@ github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I= github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= +golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.16.0 h1:7eBu7KsSvFDtSXUIDbh3aqlK4DPsZ1rByC8PFfBThos= golang.org/x/net v0.16.0/go.mod h1:NxSsAGuq816PNPmqtQdLE42eU2Fs7NoRIZrHJAlaCOE= golang.org/x/oauth2 v0.13.0 h1:jDDenyj+WgFtmV3zYVoi8aE2BwtXFLWOA67ZfNWftiY= golang.org/x/oauth2 v0.13.0/go.mod h1:/JMhi4ZRXAf4HG9LiNmxvk+45+96RUlVThiH8FzNBn0= +golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= +golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.6.7 h1:FZR1q0exgwxzPzp/aF+VccGrSfxfPpkBqjIIEq3ru6c= google.golang.org/appengine v1.6.7/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= +google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= +google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= google.golang.org/protobuf v1.31.0 h1:g0LDEJHgrBl9N9r17Ru3sqWhkIx2NB67okBHPwC7hs8= google.golang.org/protobuf v1.31.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= diff --git a/metrics/internal/factory/factory.go b/metrics/internal/factory/factory.go new file mode 100644 index 0000000..3e6da3d --- /dev/null +++ b/metrics/internal/factory/factory.go @@ -0,0 +1,65 @@ +package factory + +import ( + "context" + "net/http" + "os" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/config" + "github.com/mozilla-services/rapid-release-model/metrics/internal/export" + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" + "github.com/shurcooL/githubv4" + "golang.org/x/oauth2" +) + +// Factory is used to create dependencies for the CLI application +type Factory struct { + NewEncoder func() (export.Encoder, error) + NewExporter func() (export.Exporter, error) + NewGitHubHTTPClient func() (*http.Client, error) + NewGitHubGraphQLClient func() (github.GraphQLClient, error) + NewGitHubRepo func() (*github.Repo, error) +} + +// NewFactory creates the default Factory for the CLI application +func NewFactory(ctx context.Context) *Factory { + f := new(Factory) + + f.NewEncoder = func() (export.Encoder, error) { + return &export.JSONEcoder{}, nil + } + + f.NewExporter = func() (export.Exporter, error) { + encoder, err := f.NewEncoder() + if err != nil { + return nil, err + } + return &export.WriterExporter{W: os.Stdout, Encoder: encoder}, nil + } + + f.NewGitHubHTTPClient = func() (*http.Client, error) { + src := oauth2.StaticTokenSource( + &oauth2.Token{AccessToken: config.FromEnv("GITHUB", "TOKEN")}, + ) + return oauth2.NewClient(ctx, src), nil + } + + f.NewGitHubGraphQLClient = func() (github.GraphQLClient, error) { + httpClient, err := f.NewGitHubHTTPClient() + if err != nil { + return nil, err + } + gqlClient := githubv4.NewClient(httpClient) + return gqlClient, nil + } + + f.NewGitHubRepo = func() (*github.Repo, error) { + repo := &github.Repo{ + Owner: config.FromEnv("GITHUB", "REPO_OWNER"), + Name: config.FromEnv("GITHUB", "REPO_NAME"), + } + return repo, nil + } + + return f +} From 18466afcc57d0ce7de8bab48e54bf8011998cd59 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 13:25:46 +0100 Subject: [PATCH 06/24] fix: fix bug in env var helper --- metrics/internal/config/config.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/metrics/internal/config/config.go b/metrics/internal/config/config.go index 6c07cc0..115a375 100644 --- a/metrics/internal/config/config.go +++ b/metrics/internal/config/config.go @@ -9,7 +9,7 @@ import ( const envPrefix string = "RRM_METRICS" func FromEnv(p ...string) string { - p = append(p, envPrefix) - k := strings.Join(p, "__") - return os.Getenv(k) + parts := append([]string{envPrefix}, p...) + key := strings.Join(parts, "__") + return os.Getenv(key) } From 09386e935d568214793f92e04ca9409c91fff7d2 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 13:26:33 +0100 Subject: [PATCH 07/24] feat: update factory to check for GH API token --- metrics/internal/factory/factory.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/metrics/internal/factory/factory.go b/metrics/internal/factory/factory.go index 3e6da3d..5513e27 100644 --- a/metrics/internal/factory/factory.go +++ b/metrics/internal/factory/factory.go @@ -2,6 +2,7 @@ package factory import ( "context" + "fmt" "net/http" "os" @@ -38,9 +39,11 @@ func NewFactory(ctx context.Context) *Factory { } f.NewGitHubHTTPClient = func() (*http.Client, error) { - src := oauth2.StaticTokenSource( - &oauth2.Token{AccessToken: config.FromEnv("GITHUB", "TOKEN")}, - ) + token := config.FromEnv("GITHUB", "TOKEN") + if token == "" { + return nil, fmt.Errorf("Requires GitHub token to be set in env") + } + src := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}) return oauth2.NewClient(ctx, src), nil } From a3039037a868a120f3acbea78d00a95e9b84fdb8 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 13:57:13 +0100 Subject: [PATCH 08/24] feat: refactor CLI app and develop test framework --- metrics/cmd/github.go | 32 ++++++++ metrics/cmd/github_test.go | 26 ++++++ metrics/cmd/root.go | 59 ++------------ metrics/cmd/root_test.go | 127 ++++-------------------------- metrics/internal/config/config.go | 8 +- metrics/internal/test/cmd.go | 37 +++++++++ metrics/internal/test/fixtures.go | 19 +++++ metrics/internal/test/testcase.go | 92 ++++++++++++++++++++++ 8 files changed, 235 insertions(+), 165 deletions(-) create mode 100644 metrics/cmd/github.go create mode 100644 metrics/cmd/github_test.go create mode 100644 metrics/internal/test/cmd.go create mode 100644 metrics/internal/test/fixtures.go create mode 100644 metrics/internal/test/testcase.go diff --git a/metrics/cmd/github.go b/metrics/cmd/github.go new file mode 100644 index 0000000..7a63838 --- /dev/null +++ b/metrics/cmd/github.go @@ -0,0 +1,32 @@ +package cmd + +import ( + "fmt" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" + "github.com/spf13/cobra" +) + +func newGitHubCmd(f *factory.Factory) *cobra.Command { + repo, err := f.NewGitHubRepo() + if err != nil { + panic(err) + } + + cmd := &cobra.Command{ + Use: "github", + Short: "Retrieve metrics from GitHub", + Long: "Retrieve metrics from GitHub", + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + if repo.Owner == "" || repo.Name == "" { + return fmt.Errorf("Repo.Owner and Repo.Name are required. Set env vars or pass flags.") + } + return nil + }, + } + + cmd.PersistentFlags().StringVarP(&repo.Owner, "repo-owner", "o", repo.Owner, "owner of the GitHub repo") + cmd.PersistentFlags().StringVarP(&repo.Name, "repo-name", "n", repo.Name, "name of the GitHub repo") + + return cmd +} diff --git a/metrics/cmd/github_test.go b/metrics/cmd/github_test.go new file mode 100644 index 0000000..c7d654d --- /dev/null +++ b/metrics/cmd/github_test.go @@ -0,0 +1,26 @@ +package cmd + +import ( + "testing" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/config" + "github.com/mozilla-services/rapid-release-model/metrics/internal/test" +) + +func TestGitHub(t *testing.T) { + env := map[string]string{ + config.Key("GITHUB", "REPO_OWNER"): "", + config.Key("GITHUB", "REPO_NAME"): "", + } + + tests := []test.TestCase{ + { + Name: "github", + Args: []string{"github"}, + ErrContains: "", + Env: env, + }, + } + + test.RunTests(t, newRootCmd, tests) +} diff --git a/metrics/cmd/root.go b/metrics/cmd/root.go index f0a58fd..ba00d83 100644 --- a/metrics/cmd/root.go +++ b/metrics/cmd/root.go @@ -1,74 +1,31 @@ package cmd import ( + "context" "fmt" - "io" "os" + "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" "github.com/spf13/cobra" ) -// Prefix for application specific environment variables -const envPrefix = "RRM_METRICS_" - -// Repo for which to retrieve metrics -type Repo struct { - Owner string - Name string -} - -// Options for the cmd -type Options struct { - Out io.Writer - Repo *Repo -} - // newRootCmd creates a new base command for the metrics CLI app -func newRootCmd(w io.Writer) *cobra.Command { - opts := &Options{ - Out: w, - Repo: &Repo{ - Owner: os.Getenv(envPrefix + "REPO_OWNER"), - Name: os.Getenv(envPrefix + "REPO_NAME"), - }, - } - +func newRootCmd(f *factory.Factory) *cobra.Command { rootCmd := &cobra.Command{ Use: "metrics", Short: "Retrieve software delivery performance metrics", Long: "Retrieve software delivery performance metrics", - PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - if opts.Repo.Owner == "" { - return fmt.Errorf("Repo.Owner is required. Set env var or pass flag.") - } - if opts.Repo.Name == "" { - return fmt.Errorf("Repo.Name is required. Set env var or pass flag.") - } - return nil - }, - RunE: func(cmd *cobra.Command, args []string) error { - return runRoot(opts) - }, } - - rootCmd.PersistentFlags().StringVarP(&opts.Repo.Owner, "repo-owner", "o", opts.Repo.Owner, "owner of the GitHub repo") - rootCmd.PersistentFlags().StringVarP(&opts.Repo.Name, "repo-name", "n", opts.Repo.Name, "name of the GitHub repo") - + rootCmd.AddCommand(newGitHubCmd(f)) return rootCmd } -// runRoot performs the action for the metrics CLI command -func runRoot(opts *Options) error { - if _, err := fmt.Fprintf(opts.Out, "Retrieving metrics for %s/%s\n", opts.Repo.Owner, opts.Repo.Name); err != nil { - return err - } - return nil -} - // Execute the CLI application and write errors to os.Stderr func Execute() { - rootCmd := newRootCmd(os.Stdout) - if err := rootCmd.Execute(); err != nil { + ctx := context.Background() + factory := factory.NewFactory(ctx) + rootCmd := newRootCmd(factory) + if err := rootCmd.ExecuteContext(ctx); err != nil { fmt.Fprintf(os.Stderr, "error: %v\n", err) os.Exit(1) } diff --git a/metrics/cmd/root_test.go b/metrics/cmd/root_test.go index 7ee22ef..b0d912f 100644 --- a/metrics/cmd/root_test.go +++ b/metrics/cmd/root_test.go @@ -1,123 +1,26 @@ package cmd import ( - "bytes" - "fmt" - "strings" "testing" - "github.com/google/go-cmp/cmp" + "github.com/mozilla-services/rapid-release-model/metrics/internal/config" + "github.com/mozilla-services/rapid-release-model/metrics/internal/test" ) -type testCase struct { - // Name of the test case - name string - - // Environment variables to be set for the test case - env map[string]string - - // Arguments to be passed to the CLI app - args []string - - // Expected output from the CLI app - output string - - // Text expected in error. Empty string means no error expected. - errContains string -} - func TestRoot(t *testing.T) { - repo := &Repo{Owner: "hackebrot", Name: "turtle"} - - t.Setenv("RRM_METRICS_REPO_OWNER", "") - t.Setenv("RRM_METRICS_REPO_NAME", "") - - tests := []testCase{{ - name: "repo__short", - args: []string{"-o", repo.Owner, "-n", repo.Name}, - output: fmt.Sprintf("Retrieving metrics for %v/%v", repo.Owner, repo.Name), - errContains: "", - }, { - name: "repo__long", - args: []string{"--repo-owner", repo.Owner, "--repo-name", repo.Name}, - output: fmt.Sprintf("Retrieving metrics for %v/%v", repo.Owner, repo.Name), - errContains: "", - }, { - name: "read_env__name", - env: map[string]string{"RRM_METRICS_REPO_NAME": "hello"}, - args: []string{"--repo-owner", repo.Owner}, - output: fmt.Sprintf("Retrieving metrics for %v/%v", repo.Owner, "hello"), - errContains: "", - }, { - name: "read_env__owner", - env: map[string]string{"RRM_METRICS_REPO_OWNER": "mozilla"}, - args: []string{"--repo-name", repo.Name}, - output: fmt.Sprintf("Retrieving metrics for %v/%v", "mozilla", repo.Name), - errContains: "", - }, { - name: "missing_value__owner", - args: []string{"--repo-name", repo.Name}, - errContains: "Repo.Owner is required", - }, { - name: "missing_value__name", - args: []string{"--repo-owner", repo.Owner}, - errContains: "Repo.Name is required", - }} - - runTests(t, tests) -} - -// executeCmd creates and executes the metrics rood cmd -func executeCmd(args []string) (string, error) { - buf := new(bytes.Buffer) - - root := newRootCmd(buf) - root.SetOut(buf) - root.SetErr(buf) - root.SetArgs(args) - - err := root.Execute() - - return buf.String(), err -} - -// runTests is a helper for table-driven tests using subtests -func runTests(t *testing.T, tests []testCase) { - t.Helper() - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - t.Logf("running: metrics %s", strings.Join(tt.args, " ")) - - if tt.env != nil { - t.Logf("using environment: %s", tt.env) - for k, v := range tt.env { - t.Setenv(k, v) - } - } - - got, err := executeCmd(tt.args) - - if tt.errContains != "" && err == nil { - t.Fatalf("cmd did not return an error. output: %v", got) - } - - if tt.errContains == "" && err != nil { - t.Fatalf("unexpected error: %s", err) - } - - if tt.errContains != "" && err != nil && !strings.Contains(err.Error(), tt.errContains) { - t.Fatalf("error did not contain message\ngot: %v\nmissing: %v", err, tt.errContains) - } - - if tt.output != "" { - tGot := strings.TrimSpace(got) - tWant := strings.TrimSpace(tt.output) + env := map[string]string{ + config.Key("GITHUB", "REPO_OWNER"): "", + config.Key("GITHUB", "REPO_NAME"): "", + } - if !cmp.Equal(tGot, tWant) { - t.Fatalf("cmd returned unexpected output\ngot: %v\nwant: %v", tGot, tWant) - } - } - }) + tests := []test.TestCase{ + { + Name: "metrics", + Args: []string{}, + ErrContains: "", + Env: env, + }, } + + test.RunTests(t, newRootCmd, tests) } diff --git a/metrics/internal/config/config.go b/metrics/internal/config/config.go index 115a375..f3d4e73 100644 --- a/metrics/internal/config/config.go +++ b/metrics/internal/config/config.go @@ -8,8 +8,12 @@ import ( // Prefix for application specific environment variables const envPrefix string = "RRM_METRICS" -func FromEnv(p ...string) string { +func Key(p ...string) string { parts := append([]string{envPrefix}, p...) - key := strings.Join(parts, "__") + return strings.Join(parts, "__") +} + +func FromEnv(p ...string) string { + key := Key(p...) return os.Getenv(key) } diff --git a/metrics/internal/test/cmd.go b/metrics/internal/test/cmd.go new file mode 100644 index 0000000..2d2a584 --- /dev/null +++ b/metrics/internal/test/cmd.go @@ -0,0 +1,37 @@ +package test + +import ( + "bytes" + "context" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/export" + "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" + "github.com/spf13/cobra" +) + +// ExecuteCmd uses the passed in function to create a command and execute it +func ExecuteCmd(newCmd func(*factory.Factory) *cobra.Command, args []string) (string, error) { + ctx := context.Background() + buf := new(bytes.Buffer) + + // Create CLI factory for the tests + factory := factory.NewFactory(ctx) + + // Overwrite NewExporter, so that we export to buf + factory.NewExporter = func() (export.Exporter, error) { + encoder, err := factory.NewEncoder() + if err != nil { + return nil, err + } + return &export.WriterExporter{W: buf, Encoder: encoder}, nil + } + + cmd := newCmd(factory) + cmd.SetOut(buf) + cmd.SetErr(buf) + cmd.SetArgs(args) + + err := cmd.ExecuteContext(ctx) + + return buf.String(), err +} diff --git a/metrics/internal/test/fixtures.go b/metrics/internal/test/fixtures.go new file mode 100644 index 0000000..271c85b --- /dev/null +++ b/metrics/internal/test/fixtures.go @@ -0,0 +1,19 @@ +package test + +import ( + "os" + "path/filepath" +) + +// Load the given file from the fixtures directory +func LoadFixture(p ...string) ([]byte, error) { + parts := append([]string{"./fixtures"}, p...) + return os.ReadFile(filepath.Join(parts...)) +} + +// Load the fixture at the given location +func NewFixture(p ...string) func() ([]byte, error) { + return func() ([]byte, error) { + return LoadFixture(p...) + } +} diff --git a/metrics/internal/test/testcase.go b/metrics/internal/test/testcase.go new file mode 100644 index 0000000..06c0212 --- /dev/null +++ b/metrics/internal/test/testcase.go @@ -0,0 +1,92 @@ +package test + +import ( + "strings" + "testing" + + "github.com/google/go-cmp/cmp" + "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" + "github.com/spf13/cobra" +) + +type TestCase struct { + // Name of the test case + Name string + + // Environment variables to be set for the test case + Env map[string]string + + // Arguments to be passed to the CLI app + Args []string + + // Expected output from the CLI app + WantText string + + // Function to load extected output + WantFixture func() ([]byte, error) + + // Text expected in error. Empty string means no error expected. + ErrContains string +} + +// RunTests is a helper function for table-driven tests using subtests +func RunTests(t *testing.T, newCmd func(*factory.Factory) *cobra.Command, tests []TestCase) { + t.Helper() + + for _, tt := range tests { + t.Run(tt.Name, func(t *testing.T) { + t.Logf("running: metrics %s", strings.Join(tt.Args, " ")) + + // Set environment variables + if tt.Env != nil { + t.Logf("using environment: %s", tt.Env) + for k, v := range tt.Env { + t.Setenv(k, v) + } + } + + // Validate testcase configuration + if (tt.ErrContains != "" && tt.WantFixture != nil) || (tt.ErrContains != "" && tt.WantText != "") { + t.Fatalf("cannot set both errContains and wantFixture or wantText") + } + + if tt.WantFixture != nil && tt.WantText != "" { + t.Fatalf("cannot set both wantFixture and wantText") + } + + // Execute the CLI cmd with the specified args + got, err := ExecuteCmd(newCmd, tt.Args) + + if tt.ErrContains != "" && err == nil { + t.Fatalf("cmd did not return an error. output: %v", got) + } + + if tt.ErrContains == "" && err != nil { + t.Fatalf("unexpected error: %s", err) + } + + if tt.ErrContains != "" && err != nil && !strings.Contains(err.Error(), tt.ErrContains) { + t.Fatalf("error did not contain message\ngot: %v\nmissing: %v", err, tt.ErrContains) + } + + want := tt.WantText + + if tt.WantFixture != nil { + fixtureData, err := tt.WantFixture() + if err != nil { + t.Fatalf("error loading fixture: %v", err) + } + want = string(fixtureData[:]) + } + + if want != "" { + tGot := strings.TrimSpace(got) + tWant := strings.TrimSpace(want) + + if !cmp.Equal(tGot, tWant) { + t.Fatalf("cmd returned unexpected output\ngot: %v\nwant: %v", tGot, tWant) + } + } + }) + } +} From 616b609d1f4baf33040c03feb469046bb7e8f600 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 19:12:12 +0100 Subject: [PATCH 09/24] test: implement fake GraphQL client for tests --- metrics/internal/test/cmd.go | 7 ++++++ metrics/internal/test/github.go | 39 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 metrics/internal/test/github.go diff --git a/metrics/internal/test/cmd.go b/metrics/internal/test/cmd.go index 2d2a584..7b45747 100644 --- a/metrics/internal/test/cmd.go +++ b/metrics/internal/test/cmd.go @@ -6,6 +6,7 @@ import ( "github.com/mozilla-services/rapid-release-model/metrics/internal/export" "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" "github.com/spf13/cobra" ) @@ -26,6 +27,12 @@ func ExecuteCmd(newCmd func(*factory.Factory) *cobra.Command, args []string) (st return &export.WriterExporter{W: buf, Encoder: encoder}, nil } + // Overwrite NewGitHubGraphQLClient to return canned responses (fixtures) + // rather than querying the live GitHub GraphQL API. + factory.NewGitHubGraphQLClient = func() (github.GraphQLClient, error) { + return &FakeGraphQLClient{}, nil + } + cmd := newCmd(factory) cmd.SetOut(buf) cmd.SetErr(buf) diff --git a/metrics/internal/test/github.go b/metrics/internal/test/github.go new file mode 100644 index 0000000..3479408 --- /dev/null +++ b/metrics/internal/test/github.go @@ -0,0 +1,39 @@ +package test + +import ( + "context" + "encoding/json" + "fmt" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" + "github.com/shurcooL/githubv4" +) + +type FakeGraphQLClient struct{} + +func (c *FakeGraphQLClient) Query(ctx context.Context, q interface{}, variables map[string]interface{}) error { + var key string + + // Update this for other GraphQL queries under test. + switch v := q.(type) { + case *github.PullRequestsQuery: + key = "prs" + default: + return fmt.Errorf("unsupported query: %+v", v) + } + + // Default filename for fixtures (first page). + filename := "query.json" + + // If endCursor is set, we need to serve the corresponding page instead. + if c := variables["endCursor"]; c != (*githubv4.String)(nil) { + filename = fmt.Sprintf("query_%s.json", c) + } + + jsonData, err := LoadFixture(key, filename) + if err != nil { + return err + } + + return json.Unmarshal(jsonData, q) +} From 2b4cab54ed4d7176e3af96deb57ce3bd851b1790 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 20:29:28 +0100 Subject: [PATCH 10/24] feat: add capability to fetch GitHub PR data --- metrics/cmd/fixtures/prs/query.json | 43 +++++++++++++++ metrics/cmd/fixtures/prs/query_abc.json | 25 +++++++++ metrics/cmd/fixtures/prs/want__default.json | 38 +++++++++++++ metrics/cmd/fixtures/prs/want__limit.json | 20 +++++++ metrics/cmd/github.go | 2 + metrics/cmd/pull_requests.go | 59 +++++++++++++++++++++ metrics/cmd/pull_requests_test.go | 32 +++++++++++ 7 files changed, 219 insertions(+) create mode 100644 metrics/cmd/fixtures/prs/query.json create mode 100644 metrics/cmd/fixtures/prs/query_abc.json create mode 100644 metrics/cmd/fixtures/prs/want__default.json create mode 100644 metrics/cmd/fixtures/prs/want__limit.json create mode 100644 metrics/cmd/pull_requests.go create mode 100644 metrics/cmd/pull_requests_test.go diff --git a/metrics/cmd/fixtures/prs/query.json b/metrics/cmd/fixtures/prs/query.json new file mode 100644 index 0000000..fb47db4 --- /dev/null +++ b/metrics/cmd/fixtures/prs/query.json @@ -0,0 +1,43 @@ +{ + "Repository": { + "Name": "turtle", + "Owner": { + "Login": "hackebrot" + }, + "PullRequests": { + "PageInfo": { + "HasNextPage": true, + "EndCursor": "abc" + }, + "Nodes": [ + { + "ID": "PULLREQUEST1", + "Number": 1, + "Title": "Set up CI/CD workflow 📦", + "CreatedAt": "2023-09-08T16:33:20Z", + "UpdatedAt": "2023-09-10T07:24:20Z", + "ClosedAt": "2023-09-10T07:24:17Z", + "MergedAt": "2023-09-10T07:24:16Z" + }, + { + "ID": "PULLREQUEST2", + "Number": 2, + "Title": "Refactor test framework 🤖", + "CreatedAt": "2023-09-08T09:18:42Z", + "UpdatedAt": "2023-09-08T09:40:23Z", + "ClosedAt": "2023-09-08T09:40:20Z", + "MergedAt": "2023-09-08T09:40:19Z" + }, + { + "ID": "PULLREQUEST3", + "Number": 3, + "Title": "Fetch deployment metrics 🚀", + "CreatedAt": "2023-10-08T08:09:38Z", + "UpdatedAt": "2023-10-08T08:58:13Z", + "ClosedAt": "2023-11-08T08:58:10Z", + "MergedAt": "2023-11-08T08:58:10Z" + } + ] + } + } +} \ No newline at end of file diff --git a/metrics/cmd/fixtures/prs/query_abc.json b/metrics/cmd/fixtures/prs/query_abc.json new file mode 100644 index 0000000..0cf89ef --- /dev/null +++ b/metrics/cmd/fixtures/prs/query_abc.json @@ -0,0 +1,25 @@ +{ + "Repository": { + "Name": "turtle", + "Owner": { + "Login": "hackebrot" + }, + "PullRequests": { + "PageInfo": { + "HasNextPage": false, + "EndCursor": null + }, + "Nodes": [ + { + "ID": "PULLREQUEST4", + "Number": 4, + "Title": "Updating Docker image 📦", + "CreatedAt": "2023-12-08T16:33:20Z", + "UpdatedAt": "2023-12-10T07:24:20Z", + "ClosedAt": "2023-12-10T07:24:17Z", + "MergedAt": "2023-12-10T07:24:16Z" + } + ] + } + } +} \ No newline at end of file diff --git a/metrics/cmd/fixtures/prs/want__default.json b/metrics/cmd/fixtures/prs/want__default.json new file mode 100644 index 0000000..49d6310 --- /dev/null +++ b/metrics/cmd/fixtures/prs/want__default.json @@ -0,0 +1,38 @@ +[ + { + "ID": "PULLREQUEST1", + "Number": 1, + "Title": "Set up CI/CD workflow 📦", + "CreatedAt": "2023-09-08T16:33:20Z", + "UpdatedAt": "2023-09-10T07:24:20Z", + "ClosedAt": "2023-09-10T07:24:17Z", + "MergedAt": "2023-09-10T07:24:16Z" + }, + { + "ID": "PULLREQUEST2", + "Number": 2, + "Title": "Refactor test framework 🤖", + "CreatedAt": "2023-09-08T09:18:42Z", + "UpdatedAt": "2023-09-08T09:40:23Z", + "ClosedAt": "2023-09-08T09:40:20Z", + "MergedAt": "2023-09-08T09:40:19Z" + }, + { + "ID": "PULLREQUEST3", + "Number": 3, + "Title": "Fetch deployment metrics 🚀", + "CreatedAt": "2023-10-08T08:09:38Z", + "UpdatedAt": "2023-10-08T08:58:13Z", + "ClosedAt": "2023-11-08T08:58:10Z", + "MergedAt": "2023-11-08T08:58:10Z" + }, + { + "ID": "PULLREQUEST4", + "Number": 4, + "Title": "Updating Docker image 📦", + "CreatedAt": "2023-12-08T16:33:20Z", + "UpdatedAt": "2023-12-10T07:24:20Z", + "ClosedAt": "2023-12-10T07:24:17Z", + "MergedAt": "2023-12-10T07:24:16Z" + } +] \ No newline at end of file diff --git a/metrics/cmd/fixtures/prs/want__limit.json b/metrics/cmd/fixtures/prs/want__limit.json new file mode 100644 index 0000000..ba65394 --- /dev/null +++ b/metrics/cmd/fixtures/prs/want__limit.json @@ -0,0 +1,20 @@ +[ + { + "ID": "PULLREQUEST1", + "Number": 1, + "Title": "Set up CI/CD workflow 📦", + "CreatedAt": "2023-09-08T16:33:20Z", + "UpdatedAt": "2023-09-10T07:24:20Z", + "ClosedAt": "2023-09-10T07:24:17Z", + "MergedAt": "2023-09-10T07:24:16Z" + }, + { + "ID": "PULLREQUEST2", + "Number": 2, + "Title": "Refactor test framework 🤖", + "CreatedAt": "2023-09-08T09:18:42Z", + "UpdatedAt": "2023-09-08T09:40:23Z", + "ClosedAt": "2023-09-08T09:40:20Z", + "MergedAt": "2023-09-08T09:40:19Z" + } +] \ No newline at end of file diff --git a/metrics/cmd/github.go b/metrics/cmd/github.go index 7a63838..108ac9e 100644 --- a/metrics/cmd/github.go +++ b/metrics/cmd/github.go @@ -28,5 +28,7 @@ func newGitHubCmd(f *factory.Factory) *cobra.Command { cmd.PersistentFlags().StringVarP(&repo.Owner, "repo-owner", "o", repo.Owner, "owner of the GitHub repo") cmd.PersistentFlags().StringVarP(&repo.Name, "repo-name", "n", repo.Name, "name of the GitHub repo") + cmd.AddCommand(newPullRequestsCmd(f)) + return cmd } diff --git a/metrics/cmd/pull_requests.go b/metrics/cmd/pull_requests.go new file mode 100644 index 0000000..f3d809b --- /dev/null +++ b/metrics/cmd/pull_requests.go @@ -0,0 +1,59 @@ +package cmd + +import ( + "context" + "fmt" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" + "github.com/spf13/cobra" +) + +type PullRequestsOptions struct { + Limit int +} + +func newPullRequestsCmd(f *factory.Factory) *cobra.Command { + opts := new(PullRequestsOptions) + + cmd := &cobra.Command{ + Use: "prs", + Short: "Retrieve data about GitHub Pull Requests", + Long: "Retrieve data about GitHub Pull Requests", + PreRunE: func(cmd *cobra.Command, args []string) error { + if opts.Limit < 1 { + return fmt.Errorf("Limit cannot be smaller than 1.") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + return runPullRequests(cmd.Parent().Context(), f, opts) + }, + } + cmd.Flags().IntVarP(&opts.Limit, "limit", "l", 10, "limit for how many PRs to fetch") + return cmd +} + +func runPullRequests(ctx context.Context, f *factory.Factory, opts *PullRequestsOptions) error { + repo, err := f.NewGitHubRepo() + if err != nil { + return err + } + + gqlClient, err := f.NewGitHubGraphQLClient() + if err != nil { + return err + } + + pullRequests, err := github.QueryPullRequests(gqlClient, repo, opts.Limit) + if err != nil { + return err + } + + exporter, err := f.NewExporter() + if err != nil { + return err + } + + return exporter.Export(pullRequests) +} diff --git a/metrics/cmd/pull_requests_test.go b/metrics/cmd/pull_requests_test.go new file mode 100644 index 0000000..a5ac780 --- /dev/null +++ b/metrics/cmd/pull_requests_test.go @@ -0,0 +1,32 @@ +package cmd + +import ( + "testing" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/config" + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" + "github.com/mozilla-services/rapid-release-model/metrics/internal/test" +) + +func TestPullRequests(t *testing.T) { + repo := &github.Repo{Owner: "hackebrot", Name: "turtl"} + + env := map[string]string{ + config.Key("GITHUB", "REPO_OWNER"): "", + config.Key("GITHUB", "REPO_NAME"): "", + } + + tests := []test.TestCase{{ + Name: "prs__default", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs"}, + WantFixture: test.NewFixture("prs", "want__default.json"), + Env: env, + }, { + Name: "prs__limit", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs", "-l", "2"}, + WantFixture: test.NewFixture("prs", "want__limit.json"), + Env: env, + }} + + test.RunTests(t, newRootCmd, tests) +} From 38e37a2d386f84cc160ecfb06ad1c986a460544b Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 8 Nov 2023 20:30:41 +0100 Subject: [PATCH 11/24] test: verify that GraphQL query variables are set correctly --- metrics/internal/test/cmd.go | 7 ++++++- metrics/internal/test/github.go | 13 ++++++++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/metrics/internal/test/cmd.go b/metrics/internal/test/cmd.go index 7b45747..32e092f 100644 --- a/metrics/internal/test/cmd.go +++ b/metrics/internal/test/cmd.go @@ -3,6 +3,7 @@ package test import ( "bytes" "context" + "fmt" "github.com/mozilla-services/rapid-release-model/metrics/internal/export" "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" @@ -30,7 +31,11 @@ func ExecuteCmd(newCmd func(*factory.Factory) *cobra.Command, args []string) (st // Overwrite NewGitHubGraphQLClient to return canned responses (fixtures) // rather than querying the live GitHub GraphQL API. factory.NewGitHubGraphQLClient = func() (github.GraphQLClient, error) { - return &FakeGraphQLClient{}, nil + repo, err := factory.NewGitHubRepo() + if err != nil { + return nil, fmt.Errorf("error creating test repo") + } + return &FakeGraphQLClient{repo: repo}, nil } cmd := newCmd(factory) diff --git a/metrics/internal/test/github.go b/metrics/internal/test/github.go index 3479408..9439757 100644 --- a/metrics/internal/test/github.go +++ b/metrics/internal/test/github.go @@ -5,13 +5,24 @@ import ( "encoding/json" "fmt" + "github.com/google/go-cmp/cmp" "github.com/mozilla-services/rapid-release-model/metrics/internal/github" "github.com/shurcooL/githubv4" ) -type FakeGraphQLClient struct{} +type FakeGraphQLClient struct { + repo *github.Repo +} func (c *FakeGraphQLClient) Query(ctx context.Context, q interface{}, variables map[string]interface{}) error { + // Verify that the query is performed for the specified GitHub repo + if reqOwner := string(variables["owner"].(githubv4.String)); !cmp.Equal(reqOwner, c.repo.Owner) { + return fmt.Errorf("Repo.Owner in query variables (%v) does not match app config (%v)", reqOwner, c.repo.Owner) + } + if reqName := string(variables["name"].(githubv4.String)); !cmp.Equal(reqName, c.repo.Name) { + return fmt.Errorf("Repo.Name in query variables (%v) does not match app config (%v)", reqName, c.repo.Name) + } + var key string // Update this for other GraphQL queries under test. From 0849d3cf94605b4c61f00879ce27446e9bebfddc Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Thu, 9 Nov 2023 11:17:19 +0100 Subject: [PATCH 12/24] fix: update repo factory with CLI flags --- metrics/cmd/github.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/metrics/cmd/github.go b/metrics/cmd/github.go index 108ac9e..127824c 100644 --- a/metrics/cmd/github.go +++ b/metrics/cmd/github.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" "github.com/spf13/cobra" ) @@ -21,6 +22,9 @@ func newGitHubCmd(f *factory.Factory) *cobra.Command { if repo.Owner == "" || repo.Name == "" { return fmt.Errorf("Repo.Owner and Repo.Name are required. Set env vars or pass flags.") } + f.NewGitHubRepo = func() (*github.Repo, error) { + return repo, nil + } return nil }, } From 3480997d3c4cd12fde15737b701d94b3eb5b24c3 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Thu, 9 Nov 2023 13:39:21 +0100 Subject: [PATCH 13/24] fix: fix pagination for Pull Requests --- metrics/cmd/fixtures/prs/query_abc.json | 2 +- metrics/cmd/pull_requests_test.go | 2 +- metrics/internal/github/pull_requests.go | 11 +++++++++-- metrics/internal/test/github.go | 8 ++++++-- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/metrics/cmd/fixtures/prs/query_abc.json b/metrics/cmd/fixtures/prs/query_abc.json index 0cf89ef..189e274 100644 --- a/metrics/cmd/fixtures/prs/query_abc.json +++ b/metrics/cmd/fixtures/prs/query_abc.json @@ -7,7 +7,7 @@ "PullRequests": { "PageInfo": { "HasNextPage": false, - "EndCursor": null + "EndCursor": "hello" }, "Nodes": [ { diff --git a/metrics/cmd/pull_requests_test.go b/metrics/cmd/pull_requests_test.go index a5ac780..8654948 100644 --- a/metrics/cmd/pull_requests_test.go +++ b/metrics/cmd/pull_requests_test.go @@ -9,7 +9,7 @@ import ( ) func TestPullRequests(t *testing.T) { - repo := &github.Repo{Owner: "hackebrot", Name: "turtl"} + repo := &github.Repo{Owner: "hackebrot", Name: "turtle"} env := map[string]string{ config.Key("GITHUB", "REPO_OWNER"): "", diff --git a/metrics/internal/github/pull_requests.go b/metrics/internal/github/pull_requests.go index 2897dda..7e76acd 100644 --- a/metrics/internal/github/pull_requests.go +++ b/metrics/internal/github/pull_requests.go @@ -30,16 +30,23 @@ type PullRequestsQuery struct { EndCursor string } Nodes []PullRequest - } `graphql:"pullRequests(states: $states, first: $limit, after: $endCursor, orderBy: $orderBy)"` + } `graphql:"pullRequests(states: $states, first: $perPage, after: $endCursor, orderBy: $orderBy)"` } `graphql:"repository(owner: $owner, name: $name)"` } // QueryPullRequests fetches information about merged PRs from the GitHub GraphQL API func QueryPullRequests(gqlClient GraphQLClient, repo *Repo, limit int) ([]PullRequest, error) { + // Values of `first` and `last` must be within 1-100. See `Node limit` in + // GitHub's GraphQL API documentation. + perPage := limit + if limit > 100 { + perPage = 100 + } + queryVariables := map[string]interface{}{ "owner": githubv4.String(repo.Owner), "name": githubv4.String(repo.Name), - "limit": githubv4.Int(limit), + "perPage": githubv4.Int(perPage), "endCursor": (*githubv4.String)(nil), // When paginating forwards, the cursor to continue. "states": []githubv4.PullRequestState{githubv4.PullRequestStateMerged}, "orderBy": githubv4.IssueOrder{Field: githubv4.IssueOrderFieldUpdatedAt, Direction: githubv4.OrderDirectionDesc}, diff --git a/metrics/internal/test/github.go b/metrics/internal/test/github.go index 9439757..e46e0db 100644 --- a/metrics/internal/test/github.go +++ b/metrics/internal/test/github.go @@ -33,10 +33,14 @@ func (c *FakeGraphQLClient) Query(ctx context.Context, q interface{}, variables return fmt.Errorf("unsupported query: %+v", v) } - // Default filename for fixtures (first page). + // Default filename for fixtures. We don't know the endCursor before we + // perform the request. As a result, the filename for the first page does + // not contain the endCursor suffix. Subsequent requests have the suffix. filename := "query.json" - // If endCursor is set, we need to serve the corresponding page instead. + // The endCursor variable is set, which means we're serving the next page. + // The `after` GraphQL parameter is set to the value of `endCursor` of the + // previous request. Add the suffix to the fixture filenamme. if c := variables["endCursor"]; c != (*githubv4.String)(nil) { filename = fmt.Sprintf("query_%s.json", c) } From 543dd5b1982c6a45da6747e9a3b6ee70092991f0 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Thu, 9 Nov 2023 15:36:30 +0100 Subject: [PATCH 14/24] feat: add capability to fetch GitHub Release data --- metrics/cmd/fixtures/releases/query.json | 36 +++++++++++ metrics/cmd/fixtures/releases/query_123.json | 26 ++++++++ .../cmd/fixtures/releases/want__default.json | 32 ++++++++++ .../cmd/fixtures/releases/want__limit.json | 12 ++++ metrics/cmd/github.go | 1 + metrics/cmd/releases.go | 59 +++++++++++++++++++ metrics/cmd/releases_test.go | 32 ++++++++++ metrics/internal/github/releases.go | 11 +++- metrics/internal/test/github.go | 2 + 9 files changed, 209 insertions(+), 2 deletions(-) create mode 100644 metrics/cmd/fixtures/releases/query.json create mode 100644 metrics/cmd/fixtures/releases/query_123.json create mode 100644 metrics/cmd/fixtures/releases/want__default.json create mode 100644 metrics/cmd/fixtures/releases/want__limit.json create mode 100644 metrics/cmd/releases.go create mode 100644 metrics/cmd/releases_test.go diff --git a/metrics/cmd/fixtures/releases/query.json b/metrics/cmd/fixtures/releases/query.json new file mode 100644 index 0000000..032c60c --- /dev/null +++ b/metrics/cmd/fixtures/releases/query.json @@ -0,0 +1,36 @@ +{ + "Repository": { + "Name": "turtle", + "Owner": { + "Login": "hackebrot" + }, + "Releases": { + "PageInfo": { + "HasNextPage": true, + "EndCursor": "123" + }, + "Nodes": [ + { + "Name": "20.1.0", + "TagName": "20.1.0", + "IsDraft": false, + "IsLatest": true, + "IsPrerelease": false, + "Description": "Description for 20.1.0", + "CreatedAt": "2020-05-04T14:55:36Z", + "PublishedAt": "2020-05-04T15:02:21Z" + }, + { + "Name": "0.2.0", + "TagName": "0.2.0", + "IsDraft": false, + "IsLatest": false, + "IsPrerelease": false, + "Description": "Description for 0.2.0", + "CreatedAt": "2019-12-15T17:35:58Z", + "PublishedAt": "2019-12-15T20:00:44Z" + } + ] + } + } +} \ No newline at end of file diff --git a/metrics/cmd/fixtures/releases/query_123.json b/metrics/cmd/fixtures/releases/query_123.json new file mode 100644 index 0000000..c7dd439 --- /dev/null +++ b/metrics/cmd/fixtures/releases/query_123.json @@ -0,0 +1,26 @@ +{ + "Repository": { + "Name": "turtle", + "Owner": { + "Login": "hackebrot" + }, + "Releases": { + "PageInfo": { + "HasNextPage": false, + "EndCursor": "world" + }, + "Nodes": [ + { + "Name": "0.1.0", + "TagName": "0.1.0", + "IsDraft": false, + "IsLatest": false, + "IsPrerelease": false, + "Description": "Description for 0.1.0", + "CreatedAt": "2018-07-13T15:23:49Z", + "PublishedAt": "2018-07-16T13:30:36Z" + } + ] + } + } +} \ No newline at end of file diff --git a/metrics/cmd/fixtures/releases/want__default.json b/metrics/cmd/fixtures/releases/want__default.json new file mode 100644 index 0000000..2232efe --- /dev/null +++ b/metrics/cmd/fixtures/releases/want__default.json @@ -0,0 +1,32 @@ +[ + { + "Name": "20.1.0", + "TagName": "20.1.0", + "IsDraft": false, + "IsLatest": true, + "IsPrerelease": false, + "Description": "Description for 20.1.0", + "CreatedAt": "2020-05-04T14:55:36Z", + "PublishedAt": "2020-05-04T15:02:21Z" + }, + { + "Name": "0.2.0", + "TagName": "0.2.0", + "IsDraft": false, + "IsLatest": false, + "IsPrerelease": false, + "Description": "Description for 0.2.0", + "CreatedAt": "2019-12-15T17:35:58Z", + "PublishedAt": "2019-12-15T20:00:44Z" + }, + { + "Name": "0.1.0", + "TagName": "0.1.0", + "IsDraft": false, + "IsLatest": false, + "IsPrerelease": false, + "Description": "Description for 0.1.0", + "CreatedAt": "2018-07-13T15:23:49Z", + "PublishedAt": "2018-07-16T13:30:36Z" + } +] \ No newline at end of file diff --git a/metrics/cmd/fixtures/releases/want__limit.json b/metrics/cmd/fixtures/releases/want__limit.json new file mode 100644 index 0000000..60dfd8d --- /dev/null +++ b/metrics/cmd/fixtures/releases/want__limit.json @@ -0,0 +1,12 @@ +[ + { + "Name": "20.1.0", + "TagName": "20.1.0", + "IsDraft": false, + "IsLatest": true, + "IsPrerelease": false, + "Description": "Description for 20.1.0", + "CreatedAt": "2020-05-04T14:55:36Z", + "PublishedAt": "2020-05-04T15:02:21Z" + } +] \ No newline at end of file diff --git a/metrics/cmd/github.go b/metrics/cmd/github.go index 127824c..66c3d6c 100644 --- a/metrics/cmd/github.go +++ b/metrics/cmd/github.go @@ -33,6 +33,7 @@ func newGitHubCmd(f *factory.Factory) *cobra.Command { cmd.PersistentFlags().StringVarP(&repo.Name, "repo-name", "n", repo.Name, "name of the GitHub repo") cmd.AddCommand(newPullRequestsCmd(f)) + cmd.AddCommand(newReleasesCmd(f)) return cmd } diff --git a/metrics/cmd/releases.go b/metrics/cmd/releases.go new file mode 100644 index 0000000..ce541ff --- /dev/null +++ b/metrics/cmd/releases.go @@ -0,0 +1,59 @@ +package cmd + +import ( + "context" + "fmt" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" + "github.com/spf13/cobra" +) + +type ReleasesOptions struct { + Limit int +} + +func newReleasesCmd(f *factory.Factory) *cobra.Command { + opts := new(ReleasesOptions) + + cmd := &cobra.Command{ + Use: "releases", + Short: "Retrieve data about GitHub Releases", + Long: "Retrieve data about GitHub Releases", + PreRunE: func(cmd *cobra.Command, args []string) error { + if opts.Limit < 1 { + return fmt.Errorf("Limit cannot be smaller than 1.") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + return runReleases(cmd.Parent().Context(), f, opts) + }, + } + cmd.Flags().IntVarP(&opts.Limit, "limit", "l", 10, "limit for how many Releases to fetch") + return cmd +} + +func runReleases(ctx context.Context, f *factory.Factory, opts *ReleasesOptions) error { + repo, err := f.NewGitHubRepo() + if err != nil { + return err + } + + gqlClient, err := f.NewGitHubGraphQLClient() + if err != nil { + return err + } + + releases, err := github.QueryReleases(gqlClient, repo, opts.Limit) + if err != nil { + return err + } + + exporter, err := f.NewExporter() + if err != nil { + return err + } + + return exporter.Export(releases) +} diff --git a/metrics/cmd/releases_test.go b/metrics/cmd/releases_test.go new file mode 100644 index 0000000..2ace978 --- /dev/null +++ b/metrics/cmd/releases_test.go @@ -0,0 +1,32 @@ +package cmd + +import ( + "testing" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/config" + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" + "github.com/mozilla-services/rapid-release-model/metrics/internal/test" +) + +func TestReleases(t *testing.T) { + repo := &github.Repo{Owner: "hackebrot", Name: "turtle"} + + env := map[string]string{ + config.Key("GITHUB", "REPO_OWNER"): "", + config.Key("GITHUB", "REPO_NAME"): "", + } + + tests := []test.TestCase{{ + Name: "releases__default", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases"}, + WantFixture: test.NewFixture("releases", "want__default.json"), + Env: env, + }, { + Name: "releases__limit", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases", "-l", "1"}, + WantFixture: test.NewFixture("releases", "want__limit.json"), + Env: env, + }} + + test.RunTests(t, newRootCmd, tests) +} diff --git a/metrics/internal/github/releases.go b/metrics/internal/github/releases.go index 03d74a8..b256f00 100644 --- a/metrics/internal/github/releases.go +++ b/metrics/internal/github/releases.go @@ -30,16 +30,23 @@ type ReleasesQuery struct { EndCursor string } Nodes []Release - } `graphql:"releases(first: $limit, after: $endCursor, orderBy: $orderBy)"` + } `graphql:"releases(first: $perPage, after: $endCursor, orderBy: $orderBy)"` } `graphql:"repository(owner: $owner, name: $name)"` } // QueryReleases fetches information about merged PRs from the GitHub GraphQL API func QueryReleases(gqlClient GraphQLClient, repo *Repo, limit int) ([]Release, error) { + // Values of `first` and `last` must be within 1-100. See `Node limit` in + // GitHub's GraphQL API documentation. + perPage := limit + if limit > 100 { + perPage = 100 + } + queryVariables := map[string]interface{}{ "owner": githubv4.String(repo.Owner), "name": githubv4.String(repo.Name), - "limit": githubv4.Int(limit), + "perPage": githubv4.Int(perPage), "endCursor": (*githubv4.String)(nil), // When paginating forwards, the cursor to continue. "orderBy": githubv4.ReleaseOrder{Field: githubv4.ReleaseOrderFieldCreatedAt, Direction: githubv4.OrderDirectionDesc}, } diff --git a/metrics/internal/test/github.go b/metrics/internal/test/github.go index e46e0db..68f9be4 100644 --- a/metrics/internal/test/github.go +++ b/metrics/internal/test/github.go @@ -29,6 +29,8 @@ func (c *FakeGraphQLClient) Query(ctx context.Context, q interface{}, variables switch v := q.(type) { case *github.PullRequestsQuery: key = "prs" + case *github.ReleasesQuery: + key = "releases" default: return fmt.Errorf("unsupported query: %+v", v) } From 679552b0b6757e4091b3c09fe8374542c1843630 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Thu, 9 Nov 2023 20:27:10 +0100 Subject: [PATCH 15/24] fix: bump cobra to v1.8.0 and EnableTraverseRunHooks --- metrics/cmd/root.go | 7 +++++++ metrics/go.mod | 2 +- metrics/go.sum | 6 +++--- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/metrics/cmd/root.go b/metrics/cmd/root.go index ba00d83..534ad98 100644 --- a/metrics/cmd/root.go +++ b/metrics/cmd/root.go @@ -30,3 +30,10 @@ func Execute() { os.Exit(1) } } + +func init() { + // New in cobra v1.8.0. See https://github.com/spf13/cobra/pull/2044 + // Run all PersistentPreRunE hooks, so we don't have to repeat factory + // configuration or CLI flags parsing in sub commands. + cobra.EnableTraverseRunHooks = true +} diff --git a/metrics/go.mod b/metrics/go.mod index a1665fb..6cbe21d 100644 --- a/metrics/go.mod +++ b/metrics/go.mod @@ -5,7 +5,7 @@ go 1.21.3 require ( github.com/google/go-cmp v0.6.0 github.com/shurcooL/githubv4 v0.0.0-20230704064427-599ae7bbf278 - github.com/spf13/cobra v1.7.0 + github.com/spf13/cobra v1.8.0 golang.org/x/oauth2 v0.13.0 ) diff --git a/metrics/go.sum b/metrics/go.sum index 7a03d64..b69f969 100644 --- a/metrics/go.sum +++ b/metrics/go.sum @@ -1,4 +1,4 @@ -github.com/cpuguy83/go-md2man/v2 v2.0.2/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= +github.com/cpuguy83/go-md2man/v2 v2.0.3/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o= github.com/golang/protobuf v1.3.1/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= @@ -13,8 +13,8 @@ github.com/shurcooL/githubv4 v0.0.0-20230704064427-599ae7bbf278 h1:kdEGVAV4sO46D github.com/shurcooL/githubv4 v0.0.0-20230704064427-599ae7bbf278/go.mod h1:zqMwyHmnN/eDOZOdiTohqIUKUrTFX62PNlu7IJdu0q8= github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466 h1:17JxqqJY66GmZVHkmAsGEkcIu0oCe3AM420QDgGwZx0= github.com/shurcooL/graphql v0.0.0-20230722043721-ed46e5a46466/go.mod h1:9dIRpgIY7hVhoqfe0/FcYp0bpInZaT7dc3BYOprrIUE= -github.com/spf13/cobra v1.7.0 h1:hyqWnYt1ZQShIddO5kBpj3vu05/++x6tJ6dg8EC572I= -github.com/spf13/cobra v1.7.0/go.mod h1:uLxZILRyS/50WlhOIKD7W6V5bgeIt+4sICxh6uRMrb0= +github.com/spf13/cobra v1.8.0 h1:7aJaZx1B85qltLMc546zn58BxxfZdR/W22ej9CFoEf0= +github.com/spf13/cobra v1.8.0/go.mod h1:WXLWApfZ71AjXPya3WOlMsY9yMs7YeiHhFVlvLyhcho= github.com/spf13/pflag v1.0.5 h1:iy+VFUOCP1a+8yFto/drg2CJ5u0yRoB7fZw3DKv/JXA= github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= From 9fe3dc271f48acd0029c0f668f8f294a181b85b0 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Thu, 9 Nov 2023 20:47:56 +0100 Subject: [PATCH 16/24] feat: add capability to export to CSV --- metrics/cmd/root.go | 32 +++++++++++ metrics/internal/export/encoder.go | 90 ++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+) diff --git a/metrics/cmd/root.go b/metrics/cmd/root.go index 534ad98..c78ac1a 100644 --- a/metrics/cmd/root.go +++ b/metrics/cmd/root.go @@ -5,18 +5,50 @@ import ( "fmt" "os" + "github.com/mozilla-services/rapid-release-model/metrics/internal/export" "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" "github.com/spf13/cobra" ) +type MetricsOptions struct { + Export struct { + Encoding string + } +} + // newRootCmd creates a new base command for the metrics CLI app func newRootCmd(f *factory.Factory) *cobra.Command { + opts := new(MetricsOptions) + rootCmd := &cobra.Command{ Use: "metrics", Short: "Retrieve software delivery performance metrics", Long: "Retrieve software delivery performance metrics", + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + switch opts.Export.Encoding { + case "json": + f.NewEncoder = func() (export.Encoder, error) { + return export.NewJSONEncoder() + } + case "csv": + f.NewEncoder = func() (export.Encoder, error) { + return export.NewCSVEncoder() + } + case "plain": + f.NewEncoder = func() (export.Encoder, error) { + return export.NewPlainEncoder() + } + default: + return fmt.Errorf("unsupported Export.Encoding. Please use 'json', 'csv', or 'plain'.") + } + return nil + }, } + + rootCmd.PersistentFlags().StringVarP(&opts.Export.Encoding, "encoding", "e", "json", "export encoding") + rootCmd.AddCommand(newGitHubCmd(f)) + return rootCmd } diff --git a/metrics/internal/export/encoder.go b/metrics/internal/export/encoder.go index 3b3f780..d75a73e 100644 --- a/metrics/internal/export/encoder.go +++ b/metrics/internal/export/encoder.go @@ -1,9 +1,14 @@ package export import ( + "encoding/csv" "encoding/json" "fmt" "io" + "strconv" + "time" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" ) // Interface for CSV, JSON and other encoders @@ -33,3 +38,88 @@ func (p *PlainEncoder) Encode(w io.Writer, v interface{}) error { func NewPlainEncoder() (*PlainEncoder, error) { return &PlainEncoder{}, nil } + +type CSVEncoder struct{} + +func (c *CSVEncoder) Encode(w io.Writer, v interface{}) error { + var records [][]string + + csvw := csv.NewWriter(w) + + switch v := v.(type) { + case []github.PullRequest: + records = PullRequestsToCSVRecords(v) + case []github.Release: + records = ReleasesToCSVRecords(v) + default: + return fmt.Errorf("unable to export type %T to CSV", v) + } + + return csvw.WriteAll(records) +} + +func NewCSVEncoder() (*CSVEncoder, error) { + return &CSVEncoder{}, nil +} + +func PullRequestsToCSVRecords(prs []github.PullRequest) [][]string { + var records [][]string + + // Add column headers to records + records = append(records, []string{ + "id", + "number", + "title", + "createdAt", + "updatedAt", + "closedAt", + "mergedAt", + }) + + // Add a record for each pull request + for _, pr := range prs { + record := []string{ + pr.ID, + strconv.Itoa(pr.Number), + pr.Title, + pr.CreatedAt.Format(time.RFC3339), + pr.UpdatedAt.Format(time.RFC3339), + pr.ClosedAt.Format(time.RFC3339), + pr.MergedAt.Format(time.RFC3339), + } + records = append(records, record) + } + return records +} + +func ReleasesToCSVRecords(rs []github.Release) [][]string { + var records [][]string + + // Add column headers to records + records = append(records, []string{ + "name", + "tagName", + "isDraft", + "isLatest", + "isPrerelease", + "description", + "createdAt", + "publishedAt", + }) + + // Add a record for each release + for _, r := range rs { + record := []string{ + r.Name, + r.TagName, + strconv.FormatBool(r.IsDraft), + strconv.FormatBool(r.IsLatest), + strconv.FormatBool(r.IsPrerelease), + r.Description, + r.CreatedAt.Format(time.RFC3339), + r.PublishedAt.Format(time.RFC3339), + } + records = append(records, record) + } + return records +} From a1c3496327401576a9dbcef73151e86b897c7f61 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Fri, 10 Nov 2023 12:27:08 +0100 Subject: [PATCH 17/24] test: add tests for CSV export --- metrics/cmd/fixtures/prs/want__default.csv | 5 +++++ metrics/cmd/fixtures/releases/want__default.csv | 4 ++++ metrics/cmd/pull_requests_test.go | 5 +++++ metrics/cmd/releases_test.go | 5 +++++ 4 files changed, 19 insertions(+) create mode 100644 metrics/cmd/fixtures/prs/want__default.csv create mode 100644 metrics/cmd/fixtures/releases/want__default.csv diff --git a/metrics/cmd/fixtures/prs/want__default.csv b/metrics/cmd/fixtures/prs/want__default.csv new file mode 100644 index 0000000..c39625d --- /dev/null +++ b/metrics/cmd/fixtures/prs/want__default.csv @@ -0,0 +1,5 @@ +id,number,title,createdAt,updatedAt,closedAt,mergedAt +PULLREQUEST1,1,Set up CI/CD workflow 📦,2023-09-08T16:33:20Z,2023-09-10T07:24:20Z,2023-09-10T07:24:17Z,2023-09-10T07:24:16Z +PULLREQUEST2,2,Refactor test framework 🤖,2023-09-08T09:18:42Z,2023-09-08T09:40:23Z,2023-09-08T09:40:20Z,2023-09-08T09:40:19Z +PULLREQUEST3,3,Fetch deployment metrics 🚀,2023-10-08T08:09:38Z,2023-10-08T08:58:13Z,2023-11-08T08:58:10Z,2023-11-08T08:58:10Z +PULLREQUEST4,4,Updating Docker image 📦,2023-12-08T16:33:20Z,2023-12-10T07:24:20Z,2023-12-10T07:24:17Z,2023-12-10T07:24:16Z \ No newline at end of file diff --git a/metrics/cmd/fixtures/releases/want__default.csv b/metrics/cmd/fixtures/releases/want__default.csv new file mode 100644 index 0000000..dfe3ad9 --- /dev/null +++ b/metrics/cmd/fixtures/releases/want__default.csv @@ -0,0 +1,4 @@ +name,tagName,isDraft,isLatest,isPrerelease,description,createdAt,publishedAt +20.1.0,20.1.0,false,true,false,Description for 20.1.0,2020-05-04T14:55:36Z,2020-05-04T15:02:21Z +0.2.0,0.2.0,false,false,false,Description for 0.2.0,2019-12-15T17:35:58Z,2019-12-15T20:00:44Z +0.1.0,0.1.0,false,false,false,Description for 0.1.0,2018-07-13T15:23:49Z,2018-07-16T13:30:36Z \ No newline at end of file diff --git a/metrics/cmd/pull_requests_test.go b/metrics/cmd/pull_requests_test.go index 8654948..ee99ad8 100644 --- a/metrics/cmd/pull_requests_test.go +++ b/metrics/cmd/pull_requests_test.go @@ -26,6 +26,11 @@ func TestPullRequests(t *testing.T) { Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs", "-l", "2"}, WantFixture: test.NewFixture("prs", "want__limit.json"), Env: env, + }, { + Name: "prs__csv", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs", "-e", "csv"}, + WantFixture: test.NewFixture("prs", "want__default.csv"), + Env: env, }} test.RunTests(t, newRootCmd, tests) diff --git a/metrics/cmd/releases_test.go b/metrics/cmd/releases_test.go index 2ace978..c7f2e34 100644 --- a/metrics/cmd/releases_test.go +++ b/metrics/cmd/releases_test.go @@ -26,6 +26,11 @@ func TestReleases(t *testing.T) { Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases", "-l", "1"}, WantFixture: test.NewFixture("releases", "want__limit.json"), Env: env, + }, { + Name: "releases__csv", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases", "-e", "csv"}, + WantFixture: test.NewFixture("releases", "want__default.csv"), + Env: env, }} test.RunTests(t, newRootCmd, tests) From b3b8605fea0a330ae0f17758ce2571cd123e4889 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Fri, 10 Nov 2023 17:17:55 +0100 Subject: [PATCH 18/24] refactor: add constructors for exporters --- metrics/internal/export/exporter.go | 22 +++++++++++++++------- metrics/internal/factory/factory.go | 2 +- metrics/internal/test/cmd.go | 2 +- 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/metrics/internal/export/exporter.go b/metrics/internal/export/exporter.go index 1af1449..44e4256 100644 --- a/metrics/internal/export/exporter.go +++ b/metrics/internal/export/exporter.go @@ -10,25 +10,33 @@ type Exporter interface { } type WriterExporter struct { - W io.Writer - Encoder Encoder + w io.Writer + encoder Encoder } func (s *WriterExporter) Export(v interface{}) error { - return s.Encoder.Encode(s.W, v) + return s.encoder.Encode(s.w, v) +} + +func NewWriterExporter(w io.Writer, e Encoder) (*WriterExporter, error) { + return &WriterExporter{w: w, encoder: e}, nil } type FileExporter struct { - Filename string - Encoder Encoder + encoder Encoder + filename string } func (f *FileExporter) Export(v interface{}) error { - file, err := os.Create(f.Filename) + file, err := os.Create(f.filename) if err != nil { return err } defer file.Close() - return f.Encoder.Encode(file, v) + return f.encoder.Encode(file, v) +} + +func NewFileExporter(e Encoder, f string) (*FileExporter, error) { + return &FileExporter{encoder: e, filename: f}, nil } diff --git a/metrics/internal/factory/factory.go b/metrics/internal/factory/factory.go index 5513e27..5c98cac 100644 --- a/metrics/internal/factory/factory.go +++ b/metrics/internal/factory/factory.go @@ -35,7 +35,7 @@ func NewFactory(ctx context.Context) *Factory { if err != nil { return nil, err } - return &export.WriterExporter{W: os.Stdout, Encoder: encoder}, nil + return export.NewWriterExporter(os.Stdout, encoder) } f.NewGitHubHTTPClient = func() (*http.Client, error) { diff --git a/metrics/internal/test/cmd.go b/metrics/internal/test/cmd.go index 32e092f..1f3c947 100644 --- a/metrics/internal/test/cmd.go +++ b/metrics/internal/test/cmd.go @@ -25,7 +25,7 @@ func ExecuteCmd(newCmd func(*factory.Factory) *cobra.Command, args []string) (st if err != nil { return nil, err } - return &export.WriterExporter{W: buf, Encoder: encoder}, nil + return export.NewWriterExporter(buf, encoder) } // Overwrite NewGitHubGraphQLClient to return canned responses (fixtures) From 853307ad673184d053025ab47491e9413e3faafc Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Mon, 13 Nov 2023 09:45:07 +0100 Subject: [PATCH 19/24] feat: add file export feature to CLI app --- metrics/cmd/pull_requests_test.go | 9 +++++++++ metrics/cmd/releases_test.go | 9 +++++++++ metrics/cmd/root.go | 13 +++++++++++++ metrics/internal/test/testcase.go | 16 ++++++++++++++++ 4 files changed, 47 insertions(+) diff --git a/metrics/cmd/pull_requests_test.go b/metrics/cmd/pull_requests_test.go index ee99ad8..c8a775d 100644 --- a/metrics/cmd/pull_requests_test.go +++ b/metrics/cmd/pull_requests_test.go @@ -1,6 +1,7 @@ package cmd import ( + "path/filepath" "testing" "github.com/mozilla-services/rapid-release-model/metrics/internal/config" @@ -16,6 +17,8 @@ func TestPullRequests(t *testing.T) { config.Key("GITHUB", "REPO_NAME"): "", } + tempDir := t.TempDir() + tests := []test.TestCase{{ Name: "prs__default", Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs"}, @@ -31,6 +34,12 @@ func TestPullRequests(t *testing.T) { Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs", "-e", "csv"}, WantFixture: test.NewFixture("prs", "want__default.csv"), Env: env, + }, { + Name: "prs__filename", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs", "-f", filepath.Join(tempDir, "prs.json")}, + WantFixture: test.NewFixture("prs", "want__default.json"), + WantFile: filepath.Join(tempDir, "prs.json"), + Env: env, }} test.RunTests(t, newRootCmd, tests) diff --git a/metrics/cmd/releases_test.go b/metrics/cmd/releases_test.go index c7f2e34..286c96e 100644 --- a/metrics/cmd/releases_test.go +++ b/metrics/cmd/releases_test.go @@ -1,6 +1,7 @@ package cmd import ( + "path/filepath" "testing" "github.com/mozilla-services/rapid-release-model/metrics/internal/config" @@ -16,6 +17,8 @@ func TestReleases(t *testing.T) { config.Key("GITHUB", "REPO_NAME"): "", } + tempDir := t.TempDir() + tests := []test.TestCase{{ Name: "releases__default", Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases"}, @@ -31,6 +34,12 @@ func TestReleases(t *testing.T) { Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases", "-e", "csv"}, WantFixture: test.NewFixture("releases", "want__default.csv"), Env: env, + }, { + Name: "releases__filename", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases", "-f", filepath.Join(tempDir, "r.json")}, + WantFixture: test.NewFixture("releases", "want__default.json"), + WantFile: filepath.Join(tempDir, "r.json"), + Env: env, }} test.RunTests(t, newRootCmd, tests) diff --git a/metrics/cmd/root.go b/metrics/cmd/root.go index c78ac1a..f20d2c2 100644 --- a/metrics/cmd/root.go +++ b/metrics/cmd/root.go @@ -13,6 +13,7 @@ import ( type MetricsOptions struct { Export struct { Encoding string + Filename string } } @@ -41,11 +42,23 @@ func newRootCmd(f *factory.Factory) *cobra.Command { default: return fmt.Errorf("unsupported Export.Encoding. Please use 'json', 'csv', or 'plain'.") } + + if opts.Export.Filename != "" { + f.NewExporter = func() (export.Exporter, error) { + encoder, err := f.NewEncoder() + if err != nil { + return nil, err + } + return export.NewFileExporter(encoder, opts.Export.Filename) + } + } + return nil }, } rootCmd.PersistentFlags().StringVarP(&opts.Export.Encoding, "encoding", "e", "json", "export encoding") + rootCmd.PersistentFlags().StringVarP(&opts.Export.Filename, "filename", "f", "", "export to file") rootCmd.AddCommand(newGitHubCmd(f)) diff --git a/metrics/internal/test/testcase.go b/metrics/internal/test/testcase.go index 06c0212..436e09e 100644 --- a/metrics/internal/test/testcase.go +++ b/metrics/internal/test/testcase.go @@ -1,6 +1,7 @@ package test import ( + "os" "strings" "testing" @@ -25,6 +26,9 @@ type TestCase struct { // Function to load extected output WantFixture func() ([]byte, error) + // Expect output to be written to this file + WantFile string + // Text expected in error. Empty string means no error expected. ErrContains string } @@ -69,6 +73,18 @@ func RunTests(t *testing.T, newCmd func(*factory.Factory) *cobra.Command, tests t.Fatalf("error did not contain message\ngot: %v\nmissing: %v", err, tt.ErrContains) } + if tt.WantFile != "" { + export, err := os.ReadFile(tt.WantFile) + if err != nil && got == "" { + t.Fatalf("CLI did not create file at %v. buffer is empty.", tt.WantFile) + } + if err != nil && got != "" { + t.Fatalf("CLI did not create file at %v. buffer:\n%v", tt.WantFile, got) + } + // Overwrite got with the contents of the exported file + got = string(export[:]) + } + want := tt.WantText if tt.WantFixture != nil { From 8e175aa8758c0e9d5df7627b6ffebedc6d23c454 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Mon, 13 Nov 2023 11:11:50 +0100 Subject: [PATCH 20/24] test: add more test cases for releases and prs --- metrics/cmd/pull_requests_test.go | 15 +++++++++++++++ metrics/cmd/releases_test.go | 15 +++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/metrics/cmd/pull_requests_test.go b/metrics/cmd/pull_requests_test.go index c8a775d..0ca93fa 100644 --- a/metrics/cmd/pull_requests_test.go +++ b/metrics/cmd/pull_requests_test.go @@ -20,6 +20,16 @@ func TestPullRequests(t *testing.T) { tempDir := t.TempDir() tests := []test.TestCase{{ + Name: "prs__repo_owner__required", + Args: []string{"github", "-n", repo.Name, "prs"}, + ErrContains: "Repo.Owner and Repo.Name are required. Set env vars or pass flags", + Env: env, + }, { + Name: "prs__repo_name__required", + Args: []string{"github", "-o", repo.Owner, "prs"}, + ErrContains: "Repo.Owner and Repo.Name are required. Set env vars or pass flags", + Env: env, + }, { Name: "prs__default", Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs"}, WantFixture: test.NewFixture("prs", "want__default.json"), @@ -29,6 +39,11 @@ func TestPullRequests(t *testing.T) { Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs", "-l", "2"}, WantFixture: test.NewFixture("prs", "want__limit.json"), Env: env, + }, { + Name: "prs__json", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs", "-e", "json"}, + WantFixture: test.NewFixture("prs", "want__default.json"), + Env: env, }, { Name: "prs__csv", Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "prs", "-e", "csv"}, diff --git a/metrics/cmd/releases_test.go b/metrics/cmd/releases_test.go index 286c96e..4586db4 100644 --- a/metrics/cmd/releases_test.go +++ b/metrics/cmd/releases_test.go @@ -20,6 +20,16 @@ func TestReleases(t *testing.T) { tempDir := t.TempDir() tests := []test.TestCase{{ + Name: "releases__repo_owner__required", + Args: []string{"github", "-n", repo.Name, "releases"}, + ErrContains: "Repo.Owner and Repo.Name are required. Set env vars or pass flags", + Env: env, + }, { + Name: "releases__repo_name__required", + Args: []string{"github", "-o", repo.Owner, "releases"}, + ErrContains: "Repo.Owner and Repo.Name are required. Set env vars or pass flags", + Env: env, + }, { Name: "releases__default", Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases"}, WantFixture: test.NewFixture("releases", "want__default.json"), @@ -29,6 +39,11 @@ func TestReleases(t *testing.T) { Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases", "-l", "1"}, WantFixture: test.NewFixture("releases", "want__limit.json"), Env: env, + }, { + Name: "releases__json", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases", "-e", "json"}, + WantFixture: test.NewFixture("releases", "want__default.json"), + Env: env, }, { Name: "releases__csv", Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "releases", "-e", "csv"}, From a5a0f71f122825b2d8c5992f994cbfd9987a6b6d Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Tue, 14 Nov 2023 17:41:46 +0100 Subject: [PATCH 21/24] feat: add support for GitHub deployments --- metrics/cmd/deployments.go | 63 +++++++++++++ metrics/cmd/deployments_test.go | 72 +++++++++++++++ metrics/cmd/fixtures/deployments/query.json | 52 +++++++++++ .../fixtures/deployments/query_abc123.json | 28 ++++++ .../fixtures/deployments/want__default.csv | 5 ++ .../fixtures/deployments/want__default.json | 50 +++++++++++ .../cmd/fixtures/deployments/want__limit.json | 26 ++++++ metrics/cmd/github.go | 1 + metrics/internal/export/encoder.go | 34 +++++++ metrics/internal/github/deployments.go | 89 +++++++++++++++++++ metrics/internal/test/cmd.go | 4 +- metrics/internal/test/github.go | 11 ++- metrics/internal/test/testcase.go | 5 +- 13 files changed, 436 insertions(+), 4 deletions(-) create mode 100644 metrics/cmd/deployments.go create mode 100644 metrics/cmd/deployments_test.go create mode 100644 metrics/cmd/fixtures/deployments/query.json create mode 100644 metrics/cmd/fixtures/deployments/query_abc123.json create mode 100644 metrics/cmd/fixtures/deployments/want__default.csv create mode 100644 metrics/cmd/fixtures/deployments/want__default.json create mode 100644 metrics/cmd/fixtures/deployments/want__limit.json create mode 100644 metrics/internal/github/deployments.go diff --git a/metrics/cmd/deployments.go b/metrics/cmd/deployments.go new file mode 100644 index 0000000..3ea5c74 --- /dev/null +++ b/metrics/cmd/deployments.go @@ -0,0 +1,63 @@ +package cmd + +import ( + "context" + "fmt" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/factory" + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" + "github.com/spf13/cobra" +) + +type DeploymentsOptions struct { + Limit int + Environments *[]string +} + +func newDeploymentsCmd(f *factory.Factory) *cobra.Command { + opts := new(DeploymentsOptions) + + cmd := &cobra.Command{ + Use: "deployments", + Short: "Retrieve data about GitHub Deployments", + Long: "Retrieve data about GitHub Deployments", + PreRunE: func(cmd *cobra.Command, args []string) error { + if opts.Limit < 1 { + return fmt.Errorf("Limit cannot be smaller than 1.") + } + return nil + }, + RunE: func(cmd *cobra.Command, args []string) error { + return runDeployments(cmd.Parent().Context(), f, opts) + }, + } + cmd.Flags().IntVarP(&opts.Limit, "limit", "l", 10, "limit for how many Deployments to fetch") + + opts.Environments = cmd.Flags().StringArray("env", nil, "multiple use for Deployment environments") + + return cmd +} + +func runDeployments(ctx context.Context, f *factory.Factory, opts *DeploymentsOptions) error { + repo, err := f.NewGitHubRepo() + if err != nil { + return err + } + + gqlClient, err := f.NewGitHubGraphQLClient() + if err != nil { + return err + } + + deployments, err := github.QueryDeployments(gqlClient, repo, opts.Limit, opts.Environments) + if err != nil { + return err + } + + exporter, err := f.NewExporter() + if err != nil { + return err + } + + return exporter.Export(deployments) +} diff --git a/metrics/cmd/deployments_test.go b/metrics/cmd/deployments_test.go new file mode 100644 index 0000000..ea19684 --- /dev/null +++ b/metrics/cmd/deployments_test.go @@ -0,0 +1,72 @@ +package cmd + +import ( + "path/filepath" + "testing" + + "github.com/mozilla-services/rapid-release-model/metrics/internal/config" + "github.com/mozilla-services/rapid-release-model/metrics/internal/github" + "github.com/mozilla-services/rapid-release-model/metrics/internal/test" + "github.com/shurcooL/githubv4" +) + +func TestDeployments(t *testing.T) { + repo := &github.Repo{Owner: "hackebrot", Name: "turtle"} + + env := map[string]string{ + config.Key("GITHUB", "REPO_OWNER"): "", + config.Key("GITHUB", "REPO_NAME"): "", + } + + tempDir := t.TempDir() + + tests := []test.TestCase{{ + Name: "deployments__repo_owner__required", + Args: []string{"github", "-n", repo.Name, "deployments"}, + ErrContains: "Repo.Owner and Repo.Name are required. Set env vars or pass flags", + Env: env, + }, { + Name: "deployments__repo_name__required", + Args: []string{"github", "-o", repo.Owner, "deployments"}, + ErrContains: "Repo.Owner and Repo.Name are required. Set env vars or pass flags", + Env: env, + }, { + Name: "deployments__default", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "deployments"}, + WantFixture: test.NewFixture("deployments", "want__default.json"), + Env: env, + }, { + Name: "deployments__limit", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "deployments", "-l", "2"}, + WantFixture: test.NewFixture("deployments", "want__limit.json"), + Env: env, + }, { + Name: "deployments__json", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "deployments", "-e", "json"}, + WantFixture: test.NewFixture("deployments", "want__default.json"), + Env: env, + }, { + Name: "deployments__csv", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "deployments", "-e", "csv"}, + WantFixture: test.NewFixture("deployments", "want__default.csv"), + Env: env, + }, { + Name: "deployments__filename", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "deployments", "-f", filepath.Join(tempDir, "r.json")}, + WantFixture: test.NewFixture("deployments", "want__default.json"), + WantFile: filepath.Join(tempDir, "r.json"), + Env: env, + }, { + Name: "deployments__env__single", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "deployments", "--env", "prod"}, + WantVariables: map[string]interface{}{"environments": []githubv4.String{githubv4.String("prod")}}, + Env: env, + }, { + Name: "deployments__env__multiple", + Args: []string{"github", "-o", repo.Owner, "-n", repo.Name, "deployments", "--env", "prod", "--env", "hello"}, + WantVariables: map[string]interface{}{"environments": []githubv4.String{githubv4.String("prod"), githubv4.String("hello")}}, + Env: env, + }} + + test.RunTests(t, newRootCmd, tests) +} diff --git a/metrics/cmd/fixtures/deployments/query.json b/metrics/cmd/fixtures/deployments/query.json new file mode 100644 index 0000000..1bad66b --- /dev/null +++ b/metrics/cmd/fixtures/deployments/query.json @@ -0,0 +1,52 @@ +{ + "Repository": { + "Name": "turtle", + "Owner": { + "Login": "hackebrot" + }, + "Deployments": { + "PageInfo": { + "HasNextPage": true, + "EndCursor": "abc123" + }, + "Nodes": [ + { + "Description": "Deployment03", + "CreatedAt": "2022-05-02T20:25:05Z", + "UpdatedAt": "2022-05-02T20:25:05Z", + "OriginalEnvironment": "prod", + "LatestEnvironment": "prod", + "Task": "deploy", + "State": "ACTIVE", + "Commit": { + "AbbreviatedOid": "1abc111" + } + }, + { + "Description": "Deployment03", + "CreatedAt": "2022-05-01T20:20:05Z", + "UpdatedAt": "2022-05-01T20:20:05Z", + "OriginalEnvironment": "stage", + "LatestEnvironment": "stage", + "Task": "deploy", + "State": "ACTIVE", + "Commit": { + "AbbreviatedOid": "1abc111" + } + }, + { + "Description": "Deployment02", + "CreatedAt": "2022-04-01T20:25:05Z", + "UpdatedAt": "2022-04-01T20:25:05Z", + "OriginalEnvironment": "stage", + "LatestEnvironment": "stage", + "Task": "deploy", + "State": "INACTIVE", + "Commit": { + "AbbreviatedOid": "2abc111" + } + } + ] + } + } +} \ No newline at end of file diff --git a/metrics/cmd/fixtures/deployments/query_abc123.json b/metrics/cmd/fixtures/deployments/query_abc123.json new file mode 100644 index 0000000..140ea01 --- /dev/null +++ b/metrics/cmd/fixtures/deployments/query_abc123.json @@ -0,0 +1,28 @@ +{ + "Repository": { + "Name": "turtle", + "Owner": { + "Login": "hackebrot" + }, + "Deployments": { + "PageInfo": { + "HasNextPage": false, + "EndCursor": "abc456" + }, + "Nodes": [ + { + "Description": "Deployment01", + "CreatedAt": "2022-02-01T20:25:05Z", + "UpdatedAt": "2022-02-01T20:25:05Z", + "OriginalEnvironment": "hello", + "LatestEnvironment": "hello", + "Task": "deploy", + "State": "ACTIVE", + "Commit": { + "AbbreviatedOid": "3abc111" + } + } + ] + } + } +} \ No newline at end of file diff --git a/metrics/cmd/fixtures/deployments/want__default.csv b/metrics/cmd/fixtures/deployments/want__default.csv new file mode 100644 index 0000000..599aacc --- /dev/null +++ b/metrics/cmd/fixtures/deployments/want__default.csv @@ -0,0 +1,5 @@ +description,createdAt,updatedAt,originalEnvironment,latestEnvironment,task,state,commitOid +Deployment03,2022-05-02T20:25:05Z,2022-05-02T20:25:05Z,prod,prod,deploy,ACTIVE,1abc111 +Deployment03,2022-05-01T20:20:05Z,2022-05-01T20:20:05Z,stage,stage,deploy,ACTIVE,1abc111 +Deployment02,2022-04-01T20:25:05Z,2022-04-01T20:25:05Z,stage,stage,deploy,INACTIVE,2abc111 +Deployment01,2022-02-01T20:25:05Z,2022-02-01T20:25:05Z,hello,hello,deploy,ACTIVE,3abc111 \ No newline at end of file diff --git a/metrics/cmd/fixtures/deployments/want__default.json b/metrics/cmd/fixtures/deployments/want__default.json new file mode 100644 index 0000000..9db8f8d --- /dev/null +++ b/metrics/cmd/fixtures/deployments/want__default.json @@ -0,0 +1,50 @@ +[ + { + "Description": "Deployment03", + "CreatedAt": "2022-05-02T20:25:05Z", + "UpdatedAt": "2022-05-02T20:25:05Z", + "OriginalEnvironment": "prod", + "LatestEnvironment": "prod", + "Task": "deploy", + "State": "ACTIVE", + "Commit": { + "AbbreviatedOid": "1abc111" + } + }, + { + "Description": "Deployment03", + "CreatedAt": "2022-05-01T20:20:05Z", + "UpdatedAt": "2022-05-01T20:20:05Z", + "OriginalEnvironment": "stage", + "LatestEnvironment": "stage", + "Task": "deploy", + "State": "ACTIVE", + "Commit": { + "AbbreviatedOid": "1abc111" + } + }, + { + "Description": "Deployment02", + "CreatedAt": "2022-04-01T20:25:05Z", + "UpdatedAt": "2022-04-01T20:25:05Z", + "OriginalEnvironment": "stage", + "LatestEnvironment": "stage", + "Task": "deploy", + "State": "INACTIVE", + "Commit": { + "AbbreviatedOid": "2abc111" + } + }, + { + "Description": "Deployment01", + "CreatedAt": "2022-02-01T20:25:05Z", + "UpdatedAt": "2022-02-01T20:25:05Z", + "OriginalEnvironment": "hello", + "LatestEnvironment": "hello", + "Task": "deploy", + "State": "ACTIVE", + "Commit": { + "AbbreviatedOid": "3abc111" + } + } +] \ No newline at end of file diff --git a/metrics/cmd/fixtures/deployments/want__limit.json b/metrics/cmd/fixtures/deployments/want__limit.json new file mode 100644 index 0000000..584d741 --- /dev/null +++ b/metrics/cmd/fixtures/deployments/want__limit.json @@ -0,0 +1,26 @@ +[ + { + "Description": "Deployment03", + "CreatedAt": "2022-05-02T20:25:05Z", + "UpdatedAt": "2022-05-02T20:25:05Z", + "OriginalEnvironment": "prod", + "LatestEnvironment": "prod", + "Task": "deploy", + "State": "ACTIVE", + "Commit": { + "AbbreviatedOid": "1abc111" + } + }, + { + "Description": "Deployment03", + "CreatedAt": "2022-05-01T20:20:05Z", + "UpdatedAt": "2022-05-01T20:20:05Z", + "OriginalEnvironment": "stage", + "LatestEnvironment": "stage", + "Task": "deploy", + "State": "ACTIVE", + "Commit": { + "AbbreviatedOid": "1abc111" + } + } +] \ No newline at end of file diff --git a/metrics/cmd/github.go b/metrics/cmd/github.go index 66c3d6c..4acf362 100644 --- a/metrics/cmd/github.go +++ b/metrics/cmd/github.go @@ -34,6 +34,7 @@ func newGitHubCmd(f *factory.Factory) *cobra.Command { cmd.AddCommand(newPullRequestsCmd(f)) cmd.AddCommand(newReleasesCmd(f)) + cmd.AddCommand(newDeploymentsCmd(f)) return cmd } diff --git a/metrics/internal/export/encoder.go b/metrics/internal/export/encoder.go index d75a73e..0dabdb1 100644 --- a/metrics/internal/export/encoder.go +++ b/metrics/internal/export/encoder.go @@ -51,6 +51,8 @@ func (c *CSVEncoder) Encode(w io.Writer, v interface{}) error { records = PullRequestsToCSVRecords(v) case []github.Release: records = ReleasesToCSVRecords(v) + case []github.Deployment: + records = DeploymentsToCSVRecords(v) default: return fmt.Errorf("unable to export type %T to CSV", v) } @@ -123,3 +125,35 @@ func ReleasesToCSVRecords(rs []github.Release) [][]string { } return records } + +func DeploymentsToCSVRecords(ds []github.Deployment) [][]string { + var records [][]string + + // Add column headers to records + records = append(records, []string{ + "description", + "createdAt", + "updatedAt", + "originalEnvironment", + "latestEnvironment", + "task", + "state", + "commitOid", + }) + + // Add a record for each deployment + for _, d := range ds { + record := []string{ + d.Description, + d.CreatedAt.Format(time.RFC3339), + d.UpdatedAt.Format(time.RFC3339), + d.OriginalEnvironment, + d.LatestEnvironment, + d.Task, + d.State, + d.Commit.AbbreviatedOid, + } + records = append(records, record) + } + return records +} diff --git a/metrics/internal/github/deployments.go b/metrics/internal/github/deployments.go new file mode 100644 index 0000000..c7e12bc --- /dev/null +++ b/metrics/internal/github/deployments.go @@ -0,0 +1,89 @@ +package github + +import ( + "context" + "time" + + "github.com/shurcooL/githubv4" +) + +type Deployment struct { + Description string + CreatedAt time.Time + UpdatedAt time.Time + OriginalEnvironment string + LatestEnvironment string + Task string + State string + Commit struct { + AbbreviatedOid string + } +} + +type DeploymentsQuery struct { + Repository struct { + Name string + Owner struct { + Login string + } + Deployments struct { + PageInfo struct { + HasNextPage bool + EndCursor string + } + Nodes []Deployment + } `graphql:"deployments(first: $perPage, after: $endCursor, orderBy: $orderBy, environments: $environments)"` + } `graphql:"repository(owner: $owner, name: $name)"` +} + +// QueryDeployments fetches information about Deployments from the GitHub GraphQL API +func QueryDeployments(gqlClient GraphQLClient, repo *Repo, limit int, envs *[]string) ([]Deployment, error) { + // Values of `first` and `last` must be within 1-100. See `Node limit` in + // GitHub's GraphQL API documentation. + perPage := limit + if limit > 100 { + perPage = 100 + } + + environments := []githubv4.String{} + + for _, e := range *envs { + environments = append(environments, githubv4.String(e)) + } + + queryVariables := map[string]interface{}{ + "owner": githubv4.String(repo.Owner), + "name": githubv4.String(repo.Name), + "perPage": githubv4.Int(perPage), + "endCursor": (*githubv4.String)(nil), // When paginating forwards, the cursor to continue. + "orderBy": githubv4.DeploymentOrder{Field: githubv4.DeploymentOrderFieldCreatedAt, Direction: githubv4.OrderDirectionDesc}, + "environments": environments, + } + + var deployments []Deployment + +Loop: + for { + var query DeploymentsQuery + + err := gqlClient.Query(context.Background(), &query, queryVariables) + if err != nil { + return nil, err + } + + for _, n := range query.Repository.Deployments.Nodes { + deployments = append(deployments, n) + if len(deployments) == limit { + break Loop + } + } + + if !query.Repository.Deployments.PageInfo.HasNextPage { + break + } + + queryVariables["endCursor"] = githubv4.String(query.Repository.Deployments.PageInfo.EndCursor) + } + + return deployments, nil +} diff --git a/metrics/internal/test/cmd.go b/metrics/internal/test/cmd.go index 1f3c947..3a4194a 100644 --- a/metrics/internal/test/cmd.go +++ b/metrics/internal/test/cmd.go @@ -12,7 +12,7 @@ import ( ) // ExecuteCmd uses the passed in function to create a command and execute it -func ExecuteCmd(newCmd func(*factory.Factory) *cobra.Command, args []string) (string, error) { +func ExecuteCmd(newCmd func(*factory.Factory) *cobra.Command, args []string, wantVariables map[string]interface{}) (string, error) { ctx := context.Background() buf := new(bytes.Buffer) @@ -35,7 +35,7 @@ func ExecuteCmd(newCmd func(*factory.Factory) *cobra.Command, args []string) (st if err != nil { return nil, fmt.Errorf("error creating test repo") } - return &FakeGraphQLClient{repo: repo}, nil + return &FakeGraphQLClient{repo: repo, wantVariables: wantVariables}, nil } cmd := newCmd(factory) diff --git a/metrics/internal/test/github.go b/metrics/internal/test/github.go index 68f9be4..5e9d630 100644 --- a/metrics/internal/test/github.go +++ b/metrics/internal/test/github.go @@ -11,7 +11,8 @@ import ( ) type FakeGraphQLClient struct { - repo *github.Repo + repo *github.Repo + wantVariables map[string]interface{} } func (c *FakeGraphQLClient) Query(ctx context.Context, q interface{}, variables map[string]interface{}) error { @@ -23,6 +24,12 @@ func (c *FakeGraphQLClient) Query(ctx context.Context, q interface{}, variables return fmt.Errorf("Repo.Name in query variables (%v) does not match app config (%v)", reqName, c.repo.Name) } + for key, want := range c.wantVariables { + if got := variables[key]; !cmp.Equal(got, want) { + return fmt.Errorf("unexpected value for GraphQL query variable %v\n%v", key, cmp.Diff(got, want)) + } + } + var key string // Update this for other GraphQL queries under test. @@ -31,6 +38,8 @@ func (c *FakeGraphQLClient) Query(ctx context.Context, q interface{}, variables key = "prs" case *github.ReleasesQuery: key = "releases" + case *github.DeploymentsQuery: + key = "deployments" default: return fmt.Errorf("unsupported query: %+v", v) } diff --git a/metrics/internal/test/testcase.go b/metrics/internal/test/testcase.go index 436e09e..e7ced50 100644 --- a/metrics/internal/test/testcase.go +++ b/metrics/internal/test/testcase.go @@ -29,6 +29,9 @@ type TestCase struct { // Expect output to be written to this file WantFile string + // Expected values for the GraphQL query variables + WantVariables map[string]interface{} + // Text expected in error. Empty string means no error expected. ErrContains string } @@ -59,7 +62,7 @@ func RunTests(t *testing.T, newCmd func(*factory.Factory) *cobra.Command, tests } // Execute the CLI cmd with the specified args - got, err := ExecuteCmd(newCmd, tt.Args) + got, err := ExecuteCmd(newCmd, tt.Args, tt.WantVariables) if tt.ErrContains != "" && err == nil { t.Fatalf("cmd did not return an error. output: %v", got) From 5ffd4c3ee84afdecb497e7418e465b0ccfd9bb81 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Tue, 14 Nov 2023 17:50:27 +0100 Subject: [PATCH 22/24] test: use cmp.Diff for test failure message --- metrics/internal/test/testcase.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/metrics/internal/test/testcase.go b/metrics/internal/test/testcase.go index e7ced50..5e5d04c 100644 --- a/metrics/internal/test/testcase.go +++ b/metrics/internal/test/testcase.go @@ -103,7 +103,7 @@ func RunTests(t *testing.T, newCmd func(*factory.Factory) *cobra.Command, tests tWant := strings.TrimSpace(want) if !cmp.Equal(tGot, tWant) { - t.Fatalf("cmd returned unexpected output\ngot: %v\nwant: %v", tGot, tWant) + t.Fatalf("cmd returned unexpected output\n%v", cmp.Diff(tGot, tWant)) } } }) From 374ed45f065b9a02ef6fbfe853728b0ada476959 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Wed, 15 Nov 2023 11:04:20 +0100 Subject: [PATCH 23/24] fix: use root context in all subcommands --- metrics/cmd/deployments.go | 2 +- metrics/cmd/pull_requests.go | 2 +- metrics/cmd/releases.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/metrics/cmd/deployments.go b/metrics/cmd/deployments.go index 3ea5c74..0c989d9 100644 --- a/metrics/cmd/deployments.go +++ b/metrics/cmd/deployments.go @@ -28,7 +28,7 @@ func newDeploymentsCmd(f *factory.Factory) *cobra.Command { return nil }, RunE: func(cmd *cobra.Command, args []string) error { - return runDeployments(cmd.Parent().Context(), f, opts) + return runDeployments(cmd.Root().Context(), f, opts) }, } cmd.Flags().IntVarP(&opts.Limit, "limit", "l", 10, "limit for how many Deployments to fetch") diff --git a/metrics/cmd/pull_requests.go b/metrics/cmd/pull_requests.go index f3d809b..3149167 100644 --- a/metrics/cmd/pull_requests.go +++ b/metrics/cmd/pull_requests.go @@ -27,7 +27,7 @@ func newPullRequestsCmd(f *factory.Factory) *cobra.Command { return nil }, RunE: func(cmd *cobra.Command, args []string) error { - return runPullRequests(cmd.Parent().Context(), f, opts) + return runPullRequests(cmd.Root().Context(), f, opts) }, } cmd.Flags().IntVarP(&opts.Limit, "limit", "l", 10, "limit for how many PRs to fetch") diff --git a/metrics/cmd/releases.go b/metrics/cmd/releases.go index ce541ff..c8f301a 100644 --- a/metrics/cmd/releases.go +++ b/metrics/cmd/releases.go @@ -27,7 +27,7 @@ func newReleasesCmd(f *factory.Factory) *cobra.Command { return nil }, RunE: func(cmd *cobra.Command, args []string) error { - return runReleases(cmd.Parent().Context(), f, opts) + return runReleases(cmd.Root().Context(), f, opts) }, } cmd.Flags().IntVarP(&opts.Limit, "limit", "l", 10, "limit for how many Releases to fetch") From be9569505fc3024238ae261bbae6e75276058d69 Mon Sep 17 00:00:00 2001 From: Raphael Pierzina Date: Thu, 23 Nov 2023 19:05:04 +0100 Subject: [PATCH 24/24] feat: report full env var key in error --- metrics/cmd/deployments_test.go | 4 ++-- metrics/cmd/github_test.go | 4 ++-- metrics/cmd/pull_requests_test.go | 4 ++-- metrics/cmd/releases_test.go | 4 ++-- metrics/cmd/root_test.go | 4 ++-- metrics/internal/config/config.go | 20 +++++++++++++++++--- metrics/internal/factory/factory.go | 10 +++++----- 7 files changed, 32 insertions(+), 18 deletions(-) diff --git a/metrics/cmd/deployments_test.go b/metrics/cmd/deployments_test.go index ea19684..1c31908 100644 --- a/metrics/cmd/deployments_test.go +++ b/metrics/cmd/deployments_test.go @@ -14,8 +14,8 @@ func TestDeployments(t *testing.T) { repo := &github.Repo{Owner: "hackebrot", Name: "turtle"} env := map[string]string{ - config.Key("GITHUB", "REPO_OWNER"): "", - config.Key("GITHUB", "REPO_NAME"): "", + config.EnvKey("GITHUB", "REPO_OWNER"): "", + config.EnvKey("GITHUB", "REPO_NAME"): "", } tempDir := t.TempDir() diff --git a/metrics/cmd/github_test.go b/metrics/cmd/github_test.go index c7d654d..51f0e92 100644 --- a/metrics/cmd/github_test.go +++ b/metrics/cmd/github_test.go @@ -9,8 +9,8 @@ import ( func TestGitHub(t *testing.T) { env := map[string]string{ - config.Key("GITHUB", "REPO_OWNER"): "", - config.Key("GITHUB", "REPO_NAME"): "", + config.EnvKey("GITHUB", "REPO_OWNER"): "", + config.EnvKey("GITHUB", "REPO_NAME"): "", } tests := []test.TestCase{ diff --git a/metrics/cmd/pull_requests_test.go b/metrics/cmd/pull_requests_test.go index 0ca93fa..b614383 100644 --- a/metrics/cmd/pull_requests_test.go +++ b/metrics/cmd/pull_requests_test.go @@ -13,8 +13,8 @@ func TestPullRequests(t *testing.T) { repo := &github.Repo{Owner: "hackebrot", Name: "turtle"} env := map[string]string{ - config.Key("GITHUB", "REPO_OWNER"): "", - config.Key("GITHUB", "REPO_NAME"): "", + config.EnvKey("GITHUB", "REPO_OWNER"): "", + config.EnvKey("GITHUB", "REPO_NAME"): "", } tempDir := t.TempDir() diff --git a/metrics/cmd/releases_test.go b/metrics/cmd/releases_test.go index 4586db4..99e0f3f 100644 --- a/metrics/cmd/releases_test.go +++ b/metrics/cmd/releases_test.go @@ -13,8 +13,8 @@ func TestReleases(t *testing.T) { repo := &github.Repo{Owner: "hackebrot", Name: "turtle"} env := map[string]string{ - config.Key("GITHUB", "REPO_OWNER"): "", - config.Key("GITHUB", "REPO_NAME"): "", + config.EnvKey("GITHUB", "REPO_OWNER"): "", + config.EnvKey("GITHUB", "REPO_NAME"): "", } tempDir := t.TempDir() diff --git a/metrics/cmd/root_test.go b/metrics/cmd/root_test.go index b0d912f..7a762f1 100644 --- a/metrics/cmd/root_test.go +++ b/metrics/cmd/root_test.go @@ -9,8 +9,8 @@ import ( func TestRoot(t *testing.T) { env := map[string]string{ - config.Key("GITHUB", "REPO_OWNER"): "", - config.Key("GITHUB", "REPO_NAME"): "", + config.EnvKey("GITHUB", "REPO_OWNER"): "", + config.EnvKey("GITHUB", "REPO_NAME"): "", } tests := []test.TestCase{ diff --git a/metrics/internal/config/config.go b/metrics/internal/config/config.go index f3d4e73..5cf6f0e 100644 --- a/metrics/internal/config/config.go +++ b/metrics/internal/config/config.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "os" "strings" ) @@ -8,12 +9,25 @@ import ( // Prefix for application specific environment variables const envPrefix string = "RRM_METRICS" -func Key(p ...string) string { +// EnvKey returns the full key for the given key parts. +func EnvKey(p ...string) string { parts := append([]string{envPrefix}, p...) return strings.Join(parts, "__") } -func FromEnv(p ...string) string { - key := Key(p...) +// ReadFromEnv reads the environment variable for the given parts. +func ReadFromEnv(p ...string) string { + key := EnvKey(p...) return os.Getenv(key) } + +// ReadFromEnvE reads the environment variable for the given parts and returns +// an error if the environment variable is not set or if the value is empty. +func ReadFromEnvE(p ...string) (string, error) { + key := EnvKey(p...) + val := os.Getenv(key) + if val == "" { + return "", fmt.Errorf("Required environment variable %v not set.", key) + } + return val, nil +} diff --git a/metrics/internal/factory/factory.go b/metrics/internal/factory/factory.go index 5c98cac..0c6e05f 100644 --- a/metrics/internal/factory/factory.go +++ b/metrics/internal/factory/factory.go @@ -39,9 +39,9 @@ func NewFactory(ctx context.Context) *Factory { } f.NewGitHubHTTPClient = func() (*http.Client, error) { - token := config.FromEnv("GITHUB", "TOKEN") - if token == "" { - return nil, fmt.Errorf("Requires GitHub token to be set in env") + token, err := config.ReadFromEnvE("GITHUB", "TOKEN") + if err != nil { + return nil, fmt.Errorf("Error creating GitHub HTTP Client: %w", err) } src := oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}) return oauth2.NewClient(ctx, src), nil @@ -58,8 +58,8 @@ func NewFactory(ctx context.Context) *Factory { f.NewGitHubRepo = func() (*github.Repo, error) { repo := &github.Repo{ - Owner: config.FromEnv("GITHUB", "REPO_OWNER"), - Name: config.FromEnv("GITHUB", "REPO_NAME"), + Owner: config.ReadFromEnv("GITHUB", "REPO_OWNER"), + Name: config.ReadFromEnv("GITHUB", "REPO_NAME"), } return repo, nil }