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

Fix go 1.15 int-to-string conversion warning in tests #5983

Merged
merged 1 commit into from
Aug 12, 2020
Merged

Conversation

aoldershaw
Copy link
Contributor

What does this PR accomplish?

Bug Fix | Feature | Documentation

Go 1.15 added automated vetting in tests for this particular case, and it's causing our unit tests to fail.

https://golang.org/doc/go1.15#vet

Changes proposed by this PR:

Properly convert integers to strings

Notes to reviewer:

Release Note

Contributor Checklist

Reviewer Checklist

  • Code reviewed
  • Tests reviewed
  • Documentation reviewed
  • Release notes reviewed
  • PR acceptance performed
  • New config flags added? Ensure that they are added to the
    BOSH and
    Helm packaging; otherwise, ignored for
    the integration
    tests

    (for example, if they are Garden configs that are not displayed in the
    --help text).

Go 1.15 added automated vetting in tests for this particular case, and
it's causing our unit tests to fail.

https://golang.org/doc/go1.15#vet

Signed-off-by: Aidan Oldershaw <aoldershaw@pivotal.io>
@aoldershaw aoldershaw added the release/no-impact This is an issue that never affected released versions (i.e. a regression caught prior to shipping). label Aug 12, 2020
Copy link
Member

@taylorsilva taylorsilva left a comment

Choose a reason for hiding this comment

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

LGTM!

@chenbh
Copy link
Contributor

chenbh commented Aug 12, 2020

There's some other stuff go vet ./... emits that we should probably fix:

# github.com/concourse/concourse/atc/metric/emitter
atc/metric/emitter/newrelic.go:216:8: using resp before checking for errors
# github.com/concourse/concourse/atc/metric/emitter_test
atc/metric/emitter/prometheus_test.go:202:9: using res before checking for errors
# github.com/concourse/concourse/atc/worker_test
atc/worker/choose_task_worker_test.go:55:9: the cancel function returned by context.WithCancel should be called, not discarded, to avoid a context leak
atc/worker/client_test.go:382:9: the cancel function returned by context.WithCancel should be called, not discarded, to avoid a context leak
# github.com/concourse/concourse/worker/runtime/integration/sample
worker/runtime/integration/sample/main.go:37:3: Fatal call has possible formatting directive %s
# github.com/concourse/concourse/topgun/k8s_test
topgun/k8s/dns_proxy_test.go:65:14: result of fmt.Errorf call not used

@taylorsilva
Copy link
Member

@chenbh maybe in a separate issue and PR? I get the same errors from go vet on go 1.14. Those errors are also currently not blocking every PR that's currently in flight 😓

@aoldershaw
Copy link
Contributor Author

We should also probably be running go vet in CI - might be worth creating an issue for that. Going to merge this now to unblock PRs, though - but don't want to lose track of this conversation!

@aoldershaw aoldershaw merged commit d0b12f8 into master Aug 12, 2020
@aoldershaw aoldershaw deleted the fix-go-115 branch August 12, 2020 14:43
@aoldershaw aoldershaw changed the title fix go 1.15 int-to-string conversion warning in tests Fix go 1.15 int-to-string conversion warning in tests Aug 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release/no-impact This is an issue that never affected released versions (i.e. a regression caught prior to shipping).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants