Skip to content

testutil: remove unnecessary flag parse#12197

Merged
alivxxx merged 5 commits intopingcap:masterfrom
alivxxx:test
Sep 17, 2019
Merged

testutil: remove unnecessary flag parse#12197
alivxxx merged 5 commits intopingcap:masterfrom
alivxxx:test

Conversation

@alivxxx
Copy link
Contributor

@alivxxx alivxxx commented Sep 16, 2019

What problem does this PR solve?

Unit test under go 1.13 will fail with flag provided but not defined: -test.testlogfile.

What is changed and how it works?

Remove flag.Parse in testutil since other flags may not have been defined.

Check List

Tests

  • Manual test (add detailed scripts or steps below)
    run make test using go 1.13 sucessfully.

Code changes

  • None

Side effects

  • None

Related changes

  • None

Release note

  • None

Copy link
Contributor

@jackysp jackysp left a comment

Choose a reason for hiding this comment

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

LGTM

@jackysp jackysp added the status/LGT1 Indicates that a PR has LGTM 1. label Sep 16, 2019
Copy link
Contributor

@eurekaka eurekaka left a comment

Choose a reason for hiding this comment

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

LGTM. CI failed, PTAL

@alivxxx
Copy link
Contributor Author

alivxxx commented Sep 16, 2019

/rebuild

@codecov
Copy link

codecov bot commented Sep 16, 2019

Codecov Report

Merging #12197 into master will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #12197   +/-   ##
===========================================
  Coverage   81.2659%   81.2659%           
===========================================
  Files           457        457           
  Lines        100245     100245           
===========================================
  Hits          81465      81465           
  Misses        12988      12988           
  Partials       5792       5792

@lysu lysu added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Sep 16, 2019
@alivxxx alivxxx requested a review from eurekaka September 16, 2019 08:13
@ngaut ngaut added the status/can-merge Indicates a PR has been approved by a committer. label Sep 16, 2019
@sre-bot
Copy link
Contributor

sre-bot commented Sep 16, 2019

Your auto merge job has been accepted, waiting for 12133

@sre-bot
Copy link
Contributor

sre-bot commented Sep 16, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Sep 16, 2019

@lamxTyler merge failed.

@jackysp
Copy link
Contributor

jackysp commented Sep 16, 2019

/merge

@sre-bot
Copy link
Contributor

sre-bot commented Sep 16, 2019

/run-all-tests

@sre-bot
Copy link
Contributor

sre-bot commented Sep 16, 2019

/run-all-tests

@alivxxx alivxxx merged commit 77df734 into pingcap:master Sep 17, 2019
@alivxxx alivxxx deleted the test branch September 17, 2019 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/test status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants