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

Fixes TestDownloadFile for go 1.14.2 #3434

Closed

Conversation

mik-dass
Copy link
Contributor

@mik-dass mik-dass commented Jun 26, 2020

What type of PR is this?

/kind bug

What does does this PR do / why we need it:

This PR fixes TestDownloadFile for go version go version go1.14.2 linux/amd64 by removing "" from the expected error string.

Which issue(s) this PR fixes:

Fixes #3433

How to test changes / Special notes to the reviewer:

make test should pass on go 1.14.2

@openshift-ci-robot openshift-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Jun 26, 2020
@mik-dass mik-dass force-pushed the fix_testDownloadFile branch from 0341b53 to 8ad48b0 Compare June 26, 2020 14:51
@amitkrout
Copy link
Contributor

@mik-dass Currently our odo release bits build on go 1.12 due to certain constraint in our internal build environment, so make sure that your change should not bring any last moment release failure surprises.

/hold

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Jun 29, 2020
@mik-dass
Copy link
Contributor Author

@mik-dass Currently our odo release bits build on go 1.12 due to certain constraint in our internal build environment, so make sure that your change should not bring any last moment release failure surprises.

Even our travis builds use 1.12+ and the tests have passed on travis.

@amitkrout
Copy link
Contributor

@mik-dass Currently our odo release bits build on go 1.12 due to certain constraint in our internal build environment, so make sure that your change should not bring any last moment release failure surprises.

Even our travis builds use 1.12+ and the tests have passed on travis.

On Travis the exact go version is go version go1.12.17 linux/amd64 (https://travis-ci.com/github/openshift/odo/jobs/353970162#L193). @mohammedzee1000 can you please confirm the go version we use in our build env.

By seeing the go version in travis, i guess your changes will work on the version we use in brew build but for double sure can you please test your change on the build env go version once @mohammedzee1000 confirms.

@mohammedzee1000
Copy link
Contributor

mohammedzee1000 commented Jun 30, 2020

@amitkrout currently it is 1.12 although new build env does provide us with go 1.13 so we should be moving to that in the near future.

I don't see any problems here as it does not seem to effect anything current. Get must no longer be present in error message in 1.14 and not having that in our check is fine as the rest of it is present. So this change is backward compatible

@mohammedzee1000
Copy link
Contributor

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. Required by Prow. label Jun 30, 2020
@kadel
Copy link
Member

kadel commented Jun 30, 2020

@mik-dass Currently our odo release bits build on go 1.12 due to certain constraint in our internal build environment, so make sure that your change should not bring any last moment release failure surprises.

/hold

Why put this PR on hold? This doesn't change the minimal required Go version.

/approve
/lgtm

Small fix. Approving and lgtming

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kadel

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

@openshift-ci-robot openshift-ci-robot added lgtm Indicates that a PR is ready to be merged. Required by Prow. approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. labels Jun 30, 2020
@kadel
Copy link
Member

kadel commented Jun 30, 2020

/retest

level=fatal msg="failed to fetch Common Manifests: failed to fetch dependency of \"Common Manifests\": failed to generate asset \"DNS Config\": getting public zone for \"origin-ci-int-aws.dev.rhcloud.com\": listing hosted zones: Throttling: Rate exceeded\n\tstatus code: 400, request id: 35be24b2-8482-4b1d-9632-92230d20af89"

@codecov
Copy link

codecov bot commented Jun 30, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3434      +/-   ##
==========================================
- Coverage   46.39%   46.36%   -0.03%     
==========================================
  Files         112      112              
  Lines       11233    11233              
==========================================
- Hits         5211     5208       -3     
- Misses       5517     5520       +3     
  Partials      505      505              
Impacted Files Coverage Δ
pkg/sync/sync.go 45.54% <0.00%> (-2.98%) ⬇️

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 edc7c33...8ad48b0. Read the comment docs.

@amitkrout
Copy link
Contributor

@mik-dass Currently our odo release bits build on go 1.12 due to certain constraint in our internal build environment, so make sure that your change should not bring any last moment release failure surprises.
/hold

Why put this PR on hold? This doesn't change the minimal required Go version.

TBH i did not see the code change for it. When i see the pr title like updating something that will be compiled on go 1.14.0, which reminds me the last time failure on our internal release env having go 1.12.0. So i though instead of reviewing the code at first place, better to get confirmation from @mohammedzee1000 if this is doable.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

10 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

19 similar comments
@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

@openshift-ci-robot
Copy link
Collaborator

openshift-ci-robot commented Jul 1, 2020

@mik-dass: The following tests failed, say /retest to rerun all failed tests:

Test name Commit Details Rerun command
ci/prow/v4.2-integration-e2e 8ad48b0 link /test v4.2-integration-e2e
ci/prow/v4.3-integration-e2e 8ad48b0 link /test v4.3-integration-e2e

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

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.

@openshift-bot
Copy link

/retest

Please review the full test history for this PR and help us cut down flakes.

* Update Node test devfile

Signed-off-by: Maysun J Faisal <maysun.j.faisal@ibm.com>

* Update node test template check in test

Signed-off-by: Maysun J Faisal <maysun.j.faisal@ibm.com>

* resolved devfile debug tests

* revert devfile vol changes

Signed-off-by: Maysun J Faisal <maysun.j.faisal@ibm.com>

* removed unnecessary git clones

* resolved serial debug test

* resolved failing docker test

Co-authored-by: Girish Ramnani <gramnani@redhat.com>
@mik-dass mik-dass force-pushed the fix_testDownloadFile branch from 8ad48b0 to 9210a18 Compare July 1, 2020 15:59
@openshift-ci-robot
Copy link
Collaborator

New changes are detected. LGTM label has been removed.

@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Jul 1, 2020
@mik-dass
Copy link
Contributor Author

mik-dass commented Jul 1, 2020

The commit 5ec8274 in #3291 resolved the issue. So this PR is not required anymore.
/close

@openshift-ci-robot
Copy link
Collaborator

@mik-dass: Closed this PR.

In response to this:

The commit 5ec8274 in #3291 resolved the issue. So this PR is not required anymore.
/close

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.

@mik-dass mik-dass deleted the fix_testDownloadFile branch July 1, 2020 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. kind/bug Categorizes issue or PR as related to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make test failing on 1.14
7 participants