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

[close #229] Add integration test coverage report for TiKV-BR #230

Merged
merged 7 commits into from
Sep 20, 2022

Conversation

haojinming
Copy link
Contributor

@haojinming haojinming commented Sep 19, 2022

What problem does this PR solve?

Issue Number: close #229

Problem Description:

What is changed and how does it work?

Add integration test coverage report for TiKV-BR

Code changes

  • Has exported function/method change

Check List for Tests

This PR has been tested by at least one of the following methods:

  • Unit test
  • Integration test
    coverage files are generated as followings:

image

Side effects

  • No side effects

Related changes

  • No related changes

Signed-off-by: haojinming <[email protected]>
Signed-off-by: haojinming <[email protected]>
@codecov
Copy link

codecov bot commented Sep 19, 2022

Codecov Report

Merging #230 (d132c49) into main (9845e58) will increase coverage by 6.5777%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@               Coverage Diff                @@
##               main       #230        +/-   ##
================================================
+ Coverage   54.4026%   60.9804%   +6.5777%     
================================================
  Files           236        238         +2     
  Lines         20181      20195        +14     
================================================
+ Hits          10979      12315      +1336     
+ Misses         8328       6758      -1570     
- Partials        874       1122       +248     
Flag Coverage Δ *Carryforward flag
br 59.8459% <ø> (+20.9107%) ⬆️
cdc 61.5023% <ø> (ø) Carriedforward from c9e1158

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
br/pkg/errors/errors.go 0.0000% <0.0000%> (ø)
br/pkg/httputil/http.go 100.0000% <0.0000%> (ø)
br/pkg/logutil/logging.go 79.3388% <0.0000%> (+1.6528%) ⬆️
br/pkg/restore/split.go 68.3098% <0.0000%> (+1.7605%) ⬆️
br/pkg/utils/safe_point.go 50.0000% <0.0000%> (+3.2258%) ⬆️
br/pkg/storage/local.go 72.7272% <0.0000%> (+3.6363%) ⬆️
br/pkg/metautil/metafile.go 63.8009% <0.0000%> (+4.5248%) ⬆️
br/pkg/storage/azblob.go 28.3524% <0.0000%> (+6.1302%) ⬆️
br/pkg/storage/s3.go 62.5310% <0.0000%> (+6.6997%) ⬆️
br/pkg/restore/range.go 62.5000% <0.0000%> (+7.5000%) ⬆️
... and 32 more

@@ -201,7 +206,11 @@ func (t *RawKVBRTester) InjectFailpoint(failpoint string) error {

func (t *RawKVBRTester) ExecBRCmd(ctx context.Context, cmdStr string) ([]byte, error) {
log.Info("exec br cmd", zap.String("br", t.br), zap.String("args", cmdStr))
cmd := exec.CommandContext(ctx, t.br, strings.Split(cmdStr, " ")...)
cmdParameter := []string{fmt.Sprintf("-test.coverprofile=%s_%s_%d", *coverageFile, t.name, t.runCnt)}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about just create a unique temporary file (see os.CreateTemp) for the coverage ? Then we don't need the case name & run count.

(If use temporary file, it may need to clear the coverage output path before running test)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks better. I have changed to temp file for coverage file. PTAL~

Copy link
Contributor

@zeminzhou zeminzhou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM~

Copy link
Collaborator

@pingyu pingyu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rest LGTM~

br/tests/rawkv/main.go Outdated Show resolved Hide resolved
Signed-off-by: haojinming <[email protected]>
@pingyu pingyu merged commit 57dc032 into tikv:main Sep 20, 2022
@haojinming haojinming deleted the br_integration_coverage branch September 20, 2022 03:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add integration test coverage report for TiKV-BR
3 participants