Skip to content

Undo the gate to do pure go test#1905

Merged
guptasu merged 1 commit intoistio:masterfrom
guptasu:undoGoTestGate
Nov 29, 2017
Merged

Undo the gate to do pure go test#1905
guptasu merged 1 commit intoistio:masterfrom
guptasu:undoGoTestGate

Conversation

@guptasu
Copy link
Copy Markdown
Contributor

@guptasu guptasu commented Nov 28, 2017

Note this would mean the checked in .gen.go files and the checked in .pb.go files can go out of sync and will not be tested for correctness.

What this PR does / why we need it:
Undo the gate to do pure go test

Which issue this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #

Special notes for your reviewer:

Release note:

none

Note this would mean the checked in .gen.go files and the checked in .pb.go files can go out of sync and will not be tested for correctness.
@codecov
Copy link
Copy Markdown

codecov bot commented Nov 28, 2017

Codecov Report

Merging #1905 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1905      +/-   ##
==========================================
+ Coverage   81.16%   81.19%   +0.02%     
==========================================
  Files         186      191       +5     
  Lines       18883    19455     +572     
==========================================
+ Hits        15327    15796     +469     
- Misses       3125     3197      +72     
- Partials      431      462      +31
Flag Coverage Δ
#broker 45.51% <ø> (+1.06%) ⬆️
#mixer 82.44% <ø> (+0.01%) ⬆️
#pilot 80.48% <ø> (+0.07%) ⬆️
#security 90.39% <ø> (ø) ⬆️
Impacted Files Coverage Δ
mixer/adapter/stackdriver/metric/bufferedClient.go 90% <0%> (-2.5%) ⬇️
pilot/proxy/envoy/resources.go 83.51% <0%> (-1.1%) ⬇️
broker/pkg/version/version.go 100% <0%> (ø)
pilot/platform/kube/inject/inject.go 88.05% <0%> (ø)
pilot/platform/kube/inject/http.go 75.86% <0%> (ø)
pilot/platform/kube/inject/initializer.go 65.41% <0%> (ø)
pilot/platform/kube/inject/configmap.go 100% <0%> (ø)
mixer/adapter/prometheus/server.go 94.91% <0%> (+5.08%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b99651d...00b2e2d. Read the comment docs.

@ZackButcher
Copy link
Copy Markdown
Contributor

/lgtm
/approve

@guptasu
Copy link
Copy Markdown
Contributor Author

guptasu commented Nov 29, 2017

/retest

Copy link
Copy Markdown
Contributor

@ayj ayj left a comment

Choose a reason for hiding this comment

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

/lgtm

@guptasu guptasu requested a review from ayj November 29, 2017 00:24
@istio-merge-robot
Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ayj, ZackButcher
We suggest the following additional approver: sebastienvas

Assign the PR to them by writing /assign @sebastienvas in a comment when ready.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@guptasu guptasu merged commit d10410a into istio:master Nov 29, 2017
@kyessenov
Copy link
Copy Markdown
Contributor

why?

@guptasu
Copy link
Copy Markdown
Contributor Author

guptasu commented Nov 29, 2017 via email

@kyessenov
Copy link
Copy Markdown
Contributor

do we do codecov with bazel now?

@guptasu
Copy link
Copy Markdown
Contributor Author

guptasu commented Nov 29, 2017 via email

@kyessenov
Copy link
Copy Markdown
Contributor

huh, that's a regression. last time i checked, bazel still can't do codecov for go, so we had to run go test anyways. it's not good if we collect wrong coverage, too.

@guptasu
Copy link
Copy Markdown
Contributor Author

guptasu commented Nov 29, 2017 via email

@kyessenov
Copy link
Copy Markdown
Contributor

yes that would work, but it's cleaner to use checked in sources rather than do symlink voodoo to replace the checked in sources with generated sources.

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.

7 participants