Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix make cover to include /exp packages #1316

Merged
merged 2 commits into from
Aug 3, 2023
Merged

Conversation

sywhang
Copy link
Contributor

@sywhang sywhang commented Aug 3, 2023

Currently make cover did not include coverage from exp/ packages because it was targeting only the root package.

This fixes the command so that it can include sub modules.

Before:

❯ make cover
go test -race -coverprofile=cover.out -coverpkg=./... ./...
?   	go.uber.org/zap/internal	[no test files]
?   	go.uber.org/zap/internal/bufferpool	[no test files]
?   	go.uber.org/zap/internal/readme	[no test files]
ok  	go.uber.org/zap	1.877s	coverage: 99.5% of statements in ./...
ok  	go.uber.org/zap/buffer	0.228s	coverage: 89.7% of statements in ./...
ok  	go.uber.org/zap/internal/color	0.353s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/exit	0.424s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/pool	0.506s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/ztest	0.566s	coverage: 15.4% of statements in ./...
ok  	go.uber.org/zap/zapcore	2.958s	coverage: 6.0% of statements in ./...
ok  	go.uber.org/zap/zapgrpc	0.524s	coverage: 8.2% of statements in ./...
ok  	go.uber.org/zap/zapio	0.560s	coverage: 5.8% of statements in ./...
ok  	go.uber.org/zap/zaptest	0.710s	coverage: 11.7% of statements in ./...
ok  	go.uber.org/zap/zaptest/observer	0.463s	coverage: 7.9% of statements in ./...
go tool cover -html=cover.out -o cover.html

After:

❯ make cover
?   	go.uber.org/zap/internal	[no test files]
?   	go.uber.org/zap/internal/bufferpool	[no test files]
?   	go.uber.org/zap/internal/readme	[no test files]
ok  	go.uber.org/zap	1.921s	coverage: 99.5% of statements in ./...
ok  	go.uber.org/zap/buffer	0.591s	coverage: 89.7% of statements in ./...
ok  	go.uber.org/zap/internal/color	0.396s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/exit	0.677s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/pool	0.929s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/ztest	0.798s	coverage: 15.4% of statements in ./...
ok  	go.uber.org/zap/zapcore	3.273s	coverage: 6.0% of statements in ./...
ok  	go.uber.org/zap/zapgrpc	0.972s	coverage: 8.2% of statements in ./...
ok  	go.uber.org/zap/zapio	1.071s	coverage: 5.8% of statements in ./...
ok  	go.uber.org/zap/zaptest	1.204s	coverage: 11.7% of statements in ./...
ok  	go.uber.org/zap/zaptest/observer	0.887s	coverage: 7.9% of statements in ./...
ok  	go.uber.org/zap/exp/zapfield	0.206s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/exp/zapslog	0.396s	coverage: 84.4% of statements in ./...
ok  	go.uber.org/zap/benchmarks	0.197s	coverage: [no statements] [no tests to run]
ok  	go.uber.org/zap/zapgrpc/internal/test	0.213s	coverage: [no statements]

Currently `make cover` did not include coverage from exp/ packages
because it was targeting only the root package.

This fixes the command so that it can include sub modules.

Before:
```
❯ make cover
go test -race -coverprofile=cover.out -coverpkg=./... ./...
?   	go.uber.org/zap/internal	[no test files]
?   	go.uber.org/zap/internal/bufferpool	[no test files]
?   	go.uber.org/zap/internal/readme	[no test files]
ok  	go.uber.org/zap	1.877s	coverage: 99.5% of statements in ./...
ok  	go.uber.org/zap/buffer	0.228s	coverage: 89.7% of statements in ./...
ok  	go.uber.org/zap/internal/color	0.353s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/exit	0.424s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/pool	0.506s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/ztest	0.566s	coverage: 15.4% of statements in ./...
ok  	go.uber.org/zap/zapcore	2.958s	coverage: 6.0% of statements in ./...
ok  	go.uber.org/zap/zapgrpc	0.524s	coverage: 8.2% of statements in ./...
ok  	go.uber.org/zap/zapio	0.560s	coverage: 5.8% of statements in ./...
ok  	go.uber.org/zap/zaptest	0.710s	coverage: 11.7% of statements in ./...
ok  	go.uber.org/zap/zaptest/observer	0.463s	coverage: 7.9% of statements in ./...
go tool cover -html=cover.out -o cover.html
```

After:
```
❯ make cover
?   	go.uber.org/zap/internal	[no test files]
?   	go.uber.org/zap/internal/bufferpool	[no test files]
?   	go.uber.org/zap/internal/readme	[no test files]
ok  	go.uber.org/zap	1.921s	coverage: 99.5% of statements in ./...
ok  	go.uber.org/zap/buffer	0.591s	coverage: 89.7% of statements in ./...
ok  	go.uber.org/zap/internal/color	0.396s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/exit	0.677s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/pool	0.929s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/internal/ztest	0.798s	coverage: 15.4% of statements in ./...
ok  	go.uber.org/zap/zapcore	3.273s	coverage: 6.0% of statements in ./...
ok  	go.uber.org/zap/zapgrpc	0.972s	coverage: 8.2% of statements in ./...
ok  	go.uber.org/zap/zapio	1.071s	coverage: 5.8% of statements in ./...
ok  	go.uber.org/zap/zaptest	1.204s	coverage: 11.7% of statements in ./...
ok  	go.uber.org/zap/zaptest/observer	0.887s	coverage: 7.9% of statements in ./...
ok  	go.uber.org/zap/exp/zapfield	0.206s	coverage: 100.0% of statements in ./...
ok  	go.uber.org/zap/exp/zapslog	0.396s	coverage: 84.4% of statements in ./...
ok  	go.uber.org/zap/benchmarks	0.197s	coverage: [no statements] [no tests to run]
ok  	go.uber.org/zap/zapgrpc/internal/test	0.213s	coverage: [no statements]
```
@sywhang sywhang requested a review from abhinav August 3, 2023 16:18
@codecov
Copy link

codecov bot commented Aug 3, 2023

Codecov Report

Merging #1316 (a2e5f59) into master (9c3c581) will decrease coverage by 0.33%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1316      +/-   ##
==========================================
- Coverage   97.93%   97.61%   -0.33%     
==========================================
  Files          50       52       +2     
  Lines        3249     3355     +106     
==========================================
+ Hits         3182     3275      +93     
- Misses         57       69      +12     
- Partials       10       11       +1     

see 2 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Makefile Outdated Show resolved Hide resolved
@sywhang sywhang merged commit 5da8736 into uber-go:master Aug 3, 2023
4 checks passed
@sywhang sywhang deleted the coverage branch August 3, 2023 17:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants