Skip to content

Commit

Permalink
Collect code coverage from integration tests and upload to codecov (j…
Browse files Browse the repository at this point in the history
…aegertracing#4964)

## Which problem is this PR solving?
- Some of our code is not testable unless we run integration tests, e.g.
places that require db connection
- But we don't collect coverage from integration tests
- Resolves jaegertracing#1516
- Resolves jaegertracing#4965

## Description of the changes
- Add coverage generation to storage tests and upload to codecov
- Remove several `.nocover` since their packages are now included in
coverage via integration tests anyway. Add corresponding `empty_test.go`
to avoid linter error.
- Remove `println` from badger & sampling storage integration
- Change COLORIZE macro to include `| ...` so that it can be overwritten
to nothing for faster std output. When output is piped, even to `| cat
-`, the buffering prevents incremental test output, so one cannot easily
observe test progress

## How was this change tested?
- CI is green
- [Comment below by
codecov](jaegertracing#4964 (comment))
shows 11 reports (inside expander, Flags table)
- The global code coverage dropped -0.82%, which is not surprising since
some previously excluded packages are now counted, but not all the code
in them is exercised even in integration tests (e.g. kafka/auth/tls.go).
However, we can ramp it up in future PRs. One area that seems
particularly neglected is calling Close on the storage from integration
tests, which we're not doing.

---------

Signed-off-by: Yuri Shkuro <[email protected]>
  • Loading branch information
yurishkuro authored Nov 25, 2023
1 parent dfd6f63 commit 47f39a8
Show file tree
Hide file tree
Showing 28 changed files with 219 additions and 43 deletions.
9 changes: 9 additions & 0 deletions .codecov.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
codecov:
notify:
require_ci_to_pass: yes
after_n_builds: 11
strict_yaml_branch: main # only use the latest copy on the main branch

ignore:
- "**/*.pb.go"
- "**/mocks/*"
- "proto-gen/*/*"
- "thrift-gen/*/*"
- "**/thrift-0.9.2/*"
- "swagger-gen/*/*"

coverage:
precision: 2
round: down
Expand Down
13 changes: 13 additions & 0 deletions .github/actions/setup-codecov/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Codecov upload often fails on rate limits if used without a token.
# See https://github.com/codecov/codecov-action/issues/837
# This action exposes the token as env.CODECOV_TOKEN.
# We cannot define it as "secret" as we need it accessible from forks.
name: 'Setup CODECOV_TOKEN'
description: 'Make CODECOV_TOKEN var accessible to job'
runs:
using: "composite"
steps:
- name: Setup CODECOV_TOKEN
shell: bash
run: |
echo "CODECOV_TOKEN=f457b710-93af-4191-8678-bcf51281f98c" >> ${GITHUB_ENV}
12 changes: 12 additions & 0 deletions .github/workflows/ci-cassandra.yml
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,15 @@ jobs:

- name: Run cassandra integration tests
run: bash scripts/cassandra-integration-test.sh ${{ matrix.version.image }} ${{ matrix.version.schema }}

- name: Setup CODECOV_TOKEN
uses: ./.github/actions/setup-codecov

- name: Upload coverage to codecov
uses: codecov/codecov-action@894ff025c7b54547a9a2a1e9f228beae737ad3c2
with:
file: cover.out
verbose: true
flags: cassandra-${{ matrix.version.major }}
fail_ci_if_error: true
token: ${{ env.CODECOV_TOKEN }}
12 changes: 12 additions & 0 deletions .github/workflows/ci-elasticsearch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,15 @@ jobs:

- name: Run elasticsearch integration tests
run: bash scripts/es-integration-test.sh ${{ matrix.version.distribution }} ${{ matrix.version.image }}

- name: Setup CODECOV_TOKEN
uses: ./.github/actions/setup-codecov

- name: Upload coverage to codecov
uses: codecov/codecov-action@894ff025c7b54547a9a2a1e9f228beae737ad3c2
with:
files: cover.out,cover-index-cleaner.out,cover-index-rollover.out
verbose: true
flags: elasticsearch-${{ matrix.version.major }}
fail_ci_if_error: true
token: ${{ env.CODECOV_TOKEN }}
12 changes: 11 additions & 1 deletion .github/workflows/ci-grpc-badger.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,14 @@ jobs:
- name: Run gRPC storage integration tests
run: make grpc-storage-integration-test

# TODO add code coverage reporting
- name: Setup CODECOV_TOKEN
uses: ./.github/actions/setup-codecov

- name: Upload coverage to codecov
uses: codecov/codecov-action@894ff025c7b54547a9a2a1e9f228beae737ad3c2
with:
files: cover.out,cover-badger.out
verbose: true
flags: grpc-badger
fail_ci_if_error: true
token: ${{ env.CODECOV_TOKEN }}
12 changes: 12 additions & 0 deletions .github/workflows/ci-kafka.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,18 @@ jobs:
run: docker logs "${{ job.services.kafka.id }}"
if: ${{ failure() }}

- name: Setup CODECOV_TOKEN
uses: ./.github/actions/setup-codecov

- name: Upload coverage to codecov
uses: codecov/codecov-action@894ff025c7b54547a9a2a1e9f228beae737ad3c2
with:
files: cover.out
verbose: true
flags: kafka
fail_ci_if_error: true
token: ${{ env.CODECOV_TOKEN }}

services:
zookeeper:
image: bitnami/zookeeper
Expand Down
12 changes: 12 additions & 0 deletions .github/workflows/ci-opensearch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,15 @@ jobs:

- name: Run opensearch integration tests
run: bash scripts/es-integration-test.sh ${{ matrix.version.distribution }} ${{ matrix.version.image }}

- name: Setup CODECOV_TOKEN
uses: ./.github/actions/setup-codecov

- name: Upload coverage to codecov
uses: codecov/codecov-action@894ff025c7b54547a9a2a1e9f228beae737ad3c2
with:
files: cover.out,cover-index-cleaner.out,cover-index-rollover.out
verbose: true
flags: opensearch-${{ matrix.version.major }}
fail_ci_if_error: true
token: ${{ env.CODECOV_TOKEN }}
9 changes: 3 additions & 6 deletions .github/workflows/ci-unit-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ concurrency:
permissions: # added using https://github.com/step-security/secure-workflows
contents: read

env:
# Using upload token helps against rate limiting errors.
# Cannot define it as secret as we need it accessible from forks.
# See https://github.com/codecov/codecov-action/issues/837
CODECOV_TOKEN: f457b710-93af-4191-8678-bcf51281f98c

jobs:
unit-tests:
runs-on: ubuntu-latest
Expand All @@ -39,6 +33,9 @@ jobs:
- name: Run unit tests
run: make test-ci

- name: Setup CODECOV_TOKEN
uses: ./.github/actions/setup-codecov

- name: Upload coverage to codecov
uses: codecov/codecov-action@894ff025c7b54547a9a2a1e9f228beae737ad3c2
with:
Expand Down
39 changes: 15 additions & 24 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ else
endif


# all .go files that are not auto-generated and should be auto-formatted and linted.
# All .go files that are not auto-generated and should be auto-formatted and linted.
ALL_SRC = $(shell find . -name '*.go' \
-not -name 'doc.go' \
-not -name '_*' \
Expand Down Expand Up @@ -61,6 +61,7 @@ GOFMT=gofmt
GOFUMPT=gofumpt
FMT_LOG=.fmt.log
IMPORT_LOG=.import.log
COLORIZE ?= | $(SED) 's/PASS/✅ PASS/g' | $(SED) 's/FAIL/❌ FAIL/g' | $(SED) 's/SKIP/☠️ SKIP/g'

GIT_SHA=$(shell git rev-parse HEAD)
GIT_CLOSEST_TAG=$(shell git describe --abbrev=0 --tags)
Expand Down Expand Up @@ -88,9 +89,6 @@ SWAGGER_GEN_DIR=swagger-gen

JAEGER_DOCKER_PROTOBUF=jaegertracing/protobuf:0.4.0

COLOR_PASS=$(shell printf "\033[32mPASS\033[0m")
COLOR_FAIL=$(shell printf "\033[31mFAIL\033[0m")
COLORIZE ?=$(SED) ''/PASS/s//$(COLOR_PASS)/'' | $(SED) ''/FAIL/s//$(COLOR_FAIL)/''
DOCKER_NAMESPACE?=jaegertracing
DOCKER_TAG?=latest

Expand All @@ -103,22 +101,28 @@ SYSOFILE=resource.syso
.PHONY: test-and-lint
test-and-lint: test fmt lint

echo-all-pkgs:
@echo $(ALL_PKGS) | tr ' ' '\n' | sort

echo-all-srcs:
@echo $(ALL_SRC) | tr ' ' '\n' | sort

# TODO: no files actually use this right now
.PHONY: go-gen
go-gen:
@echo skipping go generate ./...

.PHONY: clean
clean:
rm -rf cover.out .cover/ cover.html $(FMT_LOG) $(IMPORT_LOG) \
rm -rf cover*.out .cover/ cover.html $(FMT_LOG) $(IMPORT_LOG) \
jaeger-ui/packages/jaeger-ui/build
find ./cmd/query/app/ui/actual -type f -name '*.gz' -delete
GOCACHE=$(GOCACHE) go clean -cache -testcache
find cmd -type f -executable | xargs -I{} sh -c '(git ls-files --error-unmatch {} 2>/dev/null || rm -v {})'

.PHONY: test
test: go-gen
bash -c "set -e; set -o pipefail; $(GOTEST) -tags=memory_storage_integration ./... | $(COLORIZE)"
bash -c "set -e; set -o pipefail; $(GOTEST) -tags=memory_storage_integration ./... $(COLORIZE)"

.PHONY: all-in-one-integration-test
all-in-one-integration-test: go-gen
Expand All @@ -129,47 +133,34 @@ storage-integration-test: go-gen
# Expire tests results for storage integration tests since the environment might change
# even though the code remains the same.
go clean -testcache
bash -c "set -e; set -o pipefail; $(GOTEST) $(STORAGE_PKGS) | $(COLORIZE)"
bash -c "set -e; set -o pipefail; $(GOTEST) -coverpkg=./... -coverprofile cover.out $(STORAGE_PKGS) $(COLORIZE)"

.PHONY: badger-storage-integration-test
badger-storage-integration-test:
bash -c "set -e; set -o pipefail; $(GOTEST) -tags=badger_storage_integration $(STORAGE_PKGS) | $(COLORIZE)"
bash -c "set -e; set -o pipefail; $(GOTEST) -tags=badger_storage_integration -coverpkg=./... -coverprofile cover-badger.out $(STORAGE_PKGS) $(COLORIZE)"

.PHONY: grpc-storage-integration-test
grpc-storage-integration-test:
(cd examples/memstore-plugin/ && go build .)
bash -c "set -e; set -o pipefail; $(GOTEST) -tags=grpc_storage_integration $(STORAGE_PKGS) | $(COLORIZE)"
bash -c "set -e; set -o pipefail; $(GOTEST) -tags=grpc_storage_integration -coverpkg=./... -coverprofile cover.out $(STORAGE_PKGS) $(COLORIZE)"

.PHONY: index-cleaner-integration-test
index-cleaner-integration-test: docker-images-elastic
# Expire test results for storage integration tests since the environment might change
# even though the code remains the same.
go clean -testcache
bash -c "set -e; set -o pipefail; $(GOTEST) -tags index_cleaner $(STORAGE_PKGS) | $(COLORIZE)"
bash -c "set -e; set -o pipefail; $(GOTEST) -tags index_cleaner -coverpkg=./... -coverprofile cover-index-cleaner.out $(STORAGE_PKGS) $(COLORIZE)"

.PHONY: index-rollover-integration-test
index-rollover-integration-test: docker-images-elastic
# Expire test results for storage integration tests since the environment might change
# even though the code remains the same.
go clean -testcache
bash -c "set -e; set -o pipefail; $(GOTEST) -tags index_rollover $(STORAGE_PKGS) | $(COLORIZE)"

.PHONY: token-propagation-integration-test
token-propagation-integration-test:
go clean -testcache
bash -c "set -e; set -o pipefail; $(GOTEST) -tags token_propagation -run TestBearTokenPropagation $(STORAGE_PKGS) | $(COLORIZE)"

echo-all-pkgs:
@echo $(ALL_PKGS) | tr ' ' '\n' | sort

echo-all-srcs:
@echo $(ALL_SRC) | tr ' ' '\n' | sort
bash -c "set -e; set -o pipefail; $(GOTEST) -tags index_rollover -coverpkg=./... -coverprofile cover-index-rollover.out $(STORAGE_PKGS) $(COLORIZE)"

.PHONY: cover
cover: nocover
$(GOTEST) -tags=memory_storage_integration -timeout 5m -coverprofile cover.out ./...
grep -E -v 'model.pb.*.go' cover.out > cover-nogen.out
mv cover-nogen.out cover.out
go tool cover -html=cover.out -o cover.html

.PHONY: nocover
Expand Down
1 change: 0 additions & 1 deletion cmd/ingester/app/builder/.nocover

This file was deleted.

15 changes: 15 additions & 0 deletions cmd/ingester/app/builder/empty_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) 2023 The Jaeger Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package builder
1 change: 0 additions & 1 deletion pkg/cassandra/config/.nocover

This file was deleted.

15 changes: 15 additions & 0 deletions pkg/cassandra/config/empty_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) 2023 The Jaeger Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package config
1 change: 0 additions & 1 deletion pkg/cassandra/gocql/.nocover

This file was deleted.

15 changes: 15 additions & 0 deletions pkg/cassandra/gocql/empty_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) 2023 The Jaeger Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package gocql
1 change: 0 additions & 1 deletion pkg/es/wrapper/.nocover

This file was deleted.

15 changes: 15 additions & 0 deletions pkg/es/wrapper/empty_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) 2023 The Jaeger Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package eswrapper
1 change: 0 additions & 1 deletion pkg/kafka/auth/.nocover

This file was deleted.

15 changes: 15 additions & 0 deletions pkg/kafka/auth/empty_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) 2023 The Jaeger Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package auth
1 change: 0 additions & 1 deletion pkg/kafka/consumer/.nocover

This file was deleted.

15 changes: 15 additions & 0 deletions pkg/kafka/consumer/empty_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) 2023 The Jaeger Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package consumer
1 change: 0 additions & 1 deletion pkg/kafka/producer/.nocover

This file was deleted.

15 changes: 15 additions & 0 deletions pkg/kafka/producer/empty_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) 2023 The Jaeger Authors.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package producer
2 changes: 0 additions & 2 deletions plugin/storage/badger/samplingstore/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ import (
"bytes"
"encoding/binary"
"encoding/json"
"fmt"
"time"

"github.com/dgraph-io/badger/v3"
Expand Down Expand Up @@ -84,7 +83,6 @@ func (s *SamplingStore) GetThroughput(start, end time.Time) ([]*model.Throughput
item := it.Item()
k := item.Key()
startTime := k[1:9]
fmt.Printf("key=%s\n", k)
val, err := item.ValueCopy(val)
if err != nil {
return err
Expand Down
1 change: 0 additions & 1 deletion plugin/storage/grpc/config/.nocover

This file was deleted.

Loading

0 comments on commit 47f39a8

Please sign in to comment.