Skip to content

Commit

Permalink
Add workflows tests (#6)
Browse files Browse the repository at this point in the history
* Add workflows tests

* test golangci-lint

* Configure golangci-lint
  • Loading branch information
corentinmusard authored Apr 11, 2024
1 parent 1e2d33d commit c7a7cbc
Show file tree
Hide file tree
Showing 12 changed files with 758 additions and 66 deletions.
21 changes: 14 additions & 7 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ on:
merge_group:
branches: ["**"]

permissions:
contents: read # for golangci-lint-action

jobs:
tests:
name: Run tests
Expand All @@ -29,10 +32,14 @@ jobs:
go install github.com/jstemmer/go-junit-report@latest
- name: Build
run: go build -v ./...
#- name: Run Tests
# run: go test -v ./... | go-junit-report -set-exit-code > test-report.xml
#- name: Test Summary
# uses: test-summary/action@v2
# with:
# paths: "test-report.xml"
# if: always()
- name: Run Tests
run: go test -v ./... | go-junit-report -set-exit-code > test-report.xml
- name: Test Summary
uses: test-summary/action@v2
with:
paths: "test-report.xml"
if: always()
- name: Lint
uses: golangci/golangci-lint-action@v4
with:
version: v1.57
76 changes: 76 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# yaml-language-server: $schema=https://golangci-lint.run/jsonschema/golangci.jsonschema.json
# Inspired from: (MIT license) https://gist.github.com/maratori/47a4d00457a92aa426dbd48a18776322
---
run:
timeout: '5m'
linters-settings:
govet:
enable-all: true
disable:
- fieldalignment # too strict
- shadow # too strict
perfsprint:
strconcat: false
linters:
enable:
- asasalint # checks for pass []any as any in variadic func(...any)
- asciicheck # checks that your code does not contain non-ASCII identifiers
- bidichk # checks for dangerous unicode character sequences
- bodyclose # checks whether HTTP response body is closed successfully
- copyloopvar # detects places where loop variables are copied
#- cyclop # checks function and package cyclomatic complexity
- dupl # tool for code clone detection
- durationcheck # checks for two durations multiplied together
- errname # checks that sentinel errors are prefixed with the Err and error types are suffixed with the Error
- errorlint # finds code that will cause problems with the error wrapping scheme introduced in Go 1.13
- execinquery # checks query string in Query function which reads your Go src files and warning it finds
- exhaustive # checks exhaustiveness of enum switch statements
- exportloopref # checks for pointers to enclosing loop variables
- forbidigo # forbids identifiers
- gocheckcompilerdirectives # validates go compiler directive comments (//go:)
#- gochecknoglobals # checks that no global variables exist
- gochecknoinits # checks that no init functions are present in Go code
- gochecksumtype # checks exhaustiveness on Go "sum types"
- goconst # finds repeated strings that could be replaced by a constant
- gocritic # provides diagnostics that check for bugs, performance and style issues
- goimports # in addition to fixing imports, goimports also formats your code in the same style as gofmt
# - gomnd # detects magic numbers
- gomoddirectives # manages the use of 'replace', 'retract', and 'excludes' directives in go.mod
- gomodguard # allow and block lists linter for direct Go module dependencies. This is different from depguard where there are different block types for example version constraints and module recommendations
- goprintffuncname # checks that printf-like functions are named with f at the end
- gosec # inspects source code for security problems
- intrange # finds places where for loops could make use of an integer range
- loggercheck # checks key value pairs for common logger libraries (kitlog,klog,logr,zap)
- makezero # finds slice declarations with non-zero initial length
- mirror # reports wrong mirror patterns of bytes/strings usage
- musttag # enforces field tags in (un)marshaled structs
- nakedret # finds naked returns in functions greater than a specified function length
- nilerr # finds the code that returns nil even if it checks that the error is not nil
- nilnil # checks that there is no simultaneous return of nil error and an invalid value
- noctx # finds sending http request without context.Context
- nolintlint # reports ill-formed or insufficient nolint directives
- nonamedreturns # reports all named returns
- nosprintfhostport # checks for misuse of Sprintf to construct a host with port in a URL
- perfsprint # checks that fmt.Sprintf can be replaced with a faster alternative
- prealloc # finds slice declarations that could potentially be preallocated
- predeclared # finds code that shadows one of Go's predeclared identifiers
- promlinter # checks Prometheus metrics naming via promlint
- protogetter # reports direct reads from proto message fields when getters should be used
- reassign # checks that package variables are not reassigned
- revive # fast, configurable, extensible, flexible, and beautiful linter for Go, drop-in replacement of golint
- rowserrcheck # checks whether Err of rows is checked successfully
- sloglint # ensure consistent code style when using log/slog
- spancheck # checks for mistakes with OpenTelemetry/Census spans
- sqlclosecheck # checks that sql.Rows and sql.Stmt are closed
- stylecheck # is a replacement for golint
- tagalign # checks that struct tags are well aligned
- tenv # detects using os.Setenv instead of t.Setenv since Go1.17
- testableexamples # checks if examples are testable (have an expected output)
- testifylint # checks usage of github.com/stretchr/testify
#- testpackage # makes you use a separate _test package
- tparallel # detects inappropriate usage of t.Parallel() method in your Go test codes
- unconvert # removes unnecessary type conversions
- unparam # reports unused function parameters
- usestdlibvars # detects the possibility to use variables/constants from the Go standard library
- wastedassign # finds wasted assignment statements
- whitespace # detects leading and trailing whitespace
2 changes: 1 addition & 1 deletion generate.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
package tilebox_go
package tilebox

//go:generate go run -mod=mod github.com/bufbuild/buf/cmd/buf generate
3 changes: 2 additions & 1 deletion grpc/client_interceptor.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
package grpc

import (
"connectrpc.com/connect"
"context"

"connectrpc.com/connect"
)

type addAuthTokenInterceptor struct {
Expand Down
7 changes: 4 additions & 3 deletions grpc/grpc_connect.go
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
package grpc

import (
"connectrpc.com/connect"
"context"
"errors"
"github.com/hashicorp/go-retryablehttp"
"log/slog"
"net/http"
"net/url"
"strings"
"time"

"connectrpc.com/connect"
"github.com/hashicorp/go-retryablehttp"
)

// RetryOnStatusUnavailable provides a retry policy for retrying requests if the server is unavailable.
Expand Down Expand Up @@ -43,7 +44,7 @@ func RetryOnStatusUnavailable(ctx context.Context, resp *http.Response, err erro
return false, err
}

func RetryHttpClient() connect.HTTPClient {
func RetryHTTPClient() connect.HTTPClient {
retryClient := retryablehttp.NewClient()
retryClient.RetryWaitMin = 20 * time.Millisecond
retryClient.RetryWaitMax = 5 * time.Second
Expand Down
15 changes: 8 additions & 7 deletions observability/observability.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package observability

import (
"context"
"log/slog"

adapter "github.com/axiomhq/axiom-go/adapters/slog"
"github.com/axiomhq/axiom-go/axiom"
axiotel "github.com/axiomhq/axiom-go/axiom/otel"
Expand All @@ -13,7 +15,6 @@ import (
"go.opentelemetry.io/otel/sdk/trace"
semconv "go.opentelemetry.io/otel/semconv/v1.21.0"
oteltrace "go.opentelemetry.io/otel/trace"
"log/slog"
)

var propagator = propagation.TraceContext{}
Expand All @@ -37,15 +38,15 @@ func AxiomTraceExporter(ctx context.Context, dataset, token string) (trace.SpanE
return axiotel.TraceExporter(ctx, dataset, axiotel.SetToken(token))
}

func SetupOtelTracing(serviceName, serviceVersion string, exporters ...trace.SpanExporter) (shutdown func(ctx context.Context), err error) {
func SetupOtelTracing(serviceName, serviceVersion string, exporters ...trace.SpanExporter) func(ctx context.Context) {
tp := tracerProvider(serviceName, serviceVersion, exporters)
otel.SetTracerProvider(tp)

shutDownFunc := func(ctx context.Context) {
_ = tp.Shutdown(ctx)
}

return shutDownFunc, err
return shutDownFunc
}

// tracerProvider configures and returns a new OpenTelemetry tracer provider.
Expand Down Expand Up @@ -75,15 +76,15 @@ func GetTraceParentOfCurrentSpan(ctx context.Context) string {
return carrier.Get("traceparent")
}

func StartJobSpan[Result any](tracer oteltrace.Tracer, ctx context.Context, spanName string, job *workflowsv1.Job, f func(ctx context.Context) (Result, error)) (Result, error) {
carrier := propagation.MapCarrier{"traceparent": job.TraceParent}
func StartJobSpan[Result any](ctx context.Context, tracer oteltrace.Tracer, spanName string, job *workflowsv1.Job, f func(ctx context.Context) (Result, error)) (Result, error) {
carrier := propagation.MapCarrier{"traceparent": job.GetTraceParent()}
ctx = propagator.Extract(ctx, carrier)

return WithSpanResult(tracer, ctx, spanName, f)
return WithSpanResult(ctx, tracer, spanName, f)
}

// WithSpanResult wraps a function call that returns a result and an error with a tracing span of the given name
func WithSpanResult[Result any](tracer oteltrace.Tracer, ctx context.Context, name string, f func(ctx context.Context) (Result, error)) (Result, error) {
func WithSpanResult[Result any](ctx context.Context, tracer oteltrace.Tracer, name string, f func(ctx context.Context) (Result, error)) (Result, error) {
ctx, span := tracer.Start(ctx, name)
defer span.End()

Expand Down
8 changes: 5 additions & 3 deletions workflows/v1/jobs.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
package workflows

import (
"connectrpc.com/connect"
"context"
"encoding/json"
"errors"
"fmt"

"connectrpc.com/connect"
"github.com/tilebox/tilebox-go/observability"
workflowsv1 "github.com/tilebox/tilebox-go/protogen/go/workflows/v1"
"github.com/tilebox/tilebox-go/protogen/go/workflows/v1/workflowsv1connect"
Expand All @@ -27,7 +29,7 @@ func NewJobService(client workflowsv1connect.JobServiceClient) *JobService {

func (js *JobService) Submit(ctx context.Context, jobName, clusterSlug string, tasks ...Task) (*workflowsv1.Job, error) {
if len(tasks) == 0 {
return nil, fmt.Errorf("no tasks to submit")
return nil, errors.New("no tasks to submit")
}

rootTasks := make([]*workflowsv1.TaskSubmission, 0)
Expand Down Expand Up @@ -66,7 +68,7 @@ func (js *JobService) Submit(ctx context.Context, jobName, clusterSlug string, t
})
}

return observability.WithSpanResult(js.tracer, ctx, fmt.Sprintf("job/%s", jobName), func(ctx context.Context) (*workflowsv1.Job, error) {
return observability.WithSpanResult(ctx, js.tracer, fmt.Sprintf("job/%s", jobName), func(ctx context.Context) (*workflowsv1.Job, error) {
traceParent := observability.GetTraceParentOfCurrentSpan(ctx)

job, err := js.client.SubmitJob(ctx, connect.NewRequest(
Expand Down
89 changes: 89 additions & 0 deletions workflows/v1/jobs_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
package workflows

import (
"context"
"reflect"
"testing"

"connectrpc.com/connect"
workflowsv1 "github.com/tilebox/tilebox-go/protogen/go/workflows/v1"
"github.com/tilebox/tilebox-go/protogen/go/workflows/v1/workflowsv1connect"
)

type mockJobServiceClient struct {
workflowsv1connect.JobServiceClient
reqs []*workflowsv1.SubmitJobRequest
}

func (m *mockJobServiceClient) SubmitJob(_ context.Context, req *connect.Request[workflowsv1.SubmitJobRequest]) (*connect.Response[workflowsv1.Job], error) {
m.reqs = append(m.reqs, req.Msg)

return connect.NewResponse(&workflowsv1.Job{
Name: req.Msg.GetJobName(),
}), nil
}

func TestJobService_Submit(t *testing.T) {
ctx := context.Background()

type args struct {
jobName string
clusterSlug string
tasks []Task
}
tests := []struct {
name string
args args
want *workflowsv1.Job
wantReq *workflowsv1.SubmitJobRequest
wantErr bool
}{
{
name: "Submit Job",
args: args{
jobName: "test-job",
clusterSlug: "test-cluster",
tasks: []Task{&testTask1{}},
},
want: &workflowsv1.Job{
Name: "test-job",
},
wantReq: &workflowsv1.SubmitJobRequest{
Tasks: []*workflowsv1.TaskSubmission{
{
ClusterSlug: "test-cluster",
Identifier: &workflowsv1.TaskIdentifier{
Name: "testTask1",
Version: "v0.0",
},
Input: []byte("{\"ExecutableTask\":null}"),
Display: "testTask1",
},
},
JobName: "test-job",
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := mockJobServiceClient{}
js := NewJobService(&client)
got, err := js.Submit(ctx, tt.args.jobName, tt.args.clusterSlug, tt.args.tasks...)
if (err != nil) != tt.wantErr {
t.Errorf("Submit() error = %v, wantErr %v", err, tt.wantErr)
return
}
if !reflect.DeepEqual(got, tt.want) {
t.Errorf("Submit() got = %v, want %v", got, tt.want)
}

// Verify the submitted request
if len(client.reqs) != 1 {
t.Fatalf("Submit() expected 1 request, got %d", len(client.reqs))
}
if !reflect.DeepEqual(client.reqs[0], tt.wantReq) {
t.Errorf("Submit() request = %v, want %v", client.reqs[0], tt.wantReq)
}
})
}
}
Loading

0 comments on commit c7a7cbc

Please sign in to comment.