Skip to content

Conversation

ldemailly
Copy link
Member

@ldemailly ldemailly commented Apr 26, 2018

For issue #1038

removing the t.Skip()

should fail in CI until we have corresponding fixes

Redo of #5102

@ldemailly
Copy link
Member Author

confirmed all tests pass on master despite error with #5102 early merge

now removing the t.Skip()

@codecov
Copy link

codecov bot commented Apr 26, 2018

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #5228    +/-   ##
=======================================
+ Coverage      74%     74%    +1%     
=======================================
  Files         322     323     +1     
  Lines       27590   27145   -445     
=======================================
- Hits        20165   19934   -231     
+ Misses       6626    6427   -199     
+ Partials      799     784    -15
Impacted Files Coverage Δ
...olarwinds/internal/papertrail/papertrail_logger.go 59% <0%> (-21%) ⬇️
mixer/adapter/rbac/controller.go 39% <0%> (-16%) ⬇️
pilot/pkg/model/authentication.go 57% <0%> (-8%) ⬇️
pilot/pkg/config/clusterregistry/conversion.go 25% <0%> (-7%) ⬇️
mixer/adapter/stackdriver/log/log.go 69% <0%> (-4%) ⬇️
pilot/pkg/proxy/envoy/v2/lds.go 59% <0%> (-3%) ⬇️
pilot/pkg/proxy/envoy/v2/eds.go 77% <0%> (-3%) ⬇️
pilot/pkg/proxy/envoy/v2/ads.go 75% <0%> (-3%) ⬇️
istioctl/cmd/istioctl/convert/cmd.go 44% <0%> (-2%) ⬇️
pilot/cmd/pilot-discovery/main.go 76% <0%> (-2%) ⬇️
... and 55 more

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 23d2f39...4aa7377. Read the comment docs.

(Had to rename yes/no to oui/non because of yaml typing issue)
@ldemailly
Copy link
Member Author

until #5070 gets in master:

--- FAIL: TestAuthWithHeaders (0.67s)
	a_simple_1_test.go:221: Running with auth on yet able to connect from non istio to istio (insecure): HTTP/1.1 200 OK

(now that circle does have auth on - as expected)

@istio-testing
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ldemailly

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

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@istio-testing
Copy link
Collaborator

istio-testing commented May 18, 2018

@ldemailly: The following tests failed, say /retest to rerun them all:

Test name Commit Details Rerun command
prow/e2e-simpleTests.sh 4aa7377 link /test e2e-simple
prow/istio-pilot-e2e-envoyv2-v1alpha3.sh 4aa7377 link /test istio-pilot-e2e-envoyv2-v1alpha3
prow/istio-pilot-e2e.sh 4aa7377 link /test istio-pilot-e2e
prow/e2e-bookInfoTests-v1alpha3.sh 4aa7377 link /test e2e-bookInfo-envoyv2-v1alpha3

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@ldemailly ldemailly changed the title [test pr] 503s and auth fix check [test pr] 503s May 21, 2018
@rshriram
Copy link
Member

Can you please close this pr and recreate it from your personal fork? We are removing all personal branches from istio (per toc discussion couple of weeks ago). Everything except yours have been removed. Cc @geeknoid

@ldemailly
Copy link
Member Author

clearly the most important issues are being addressed

@ldemailly ldemailly closed this May 23, 2018
@ldemailly ldemailly deleted the 503s_test_no_skip branch May 23, 2018 20:35
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.

4 participants