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.

Sign up for GitHub

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

Jump to bottom

Fix build error in examples/go-client #3547

Merged
merged 2 commits into from
Apr 9, 2020
Merged

Fix build error in examples/go-client #3547

merged 2 commits into from
Apr 9, 2020

Conversation

awly
Copy link
Contributor

@awly awly commented Apr 8, 2020

The API of auth package changed in an incompatible way. Fix the usage.

Also, change the definition of make test to prevent this from happening.
Use go list to get the list of all packages instead of listing them manually.
Also, use the race detector with go test as a second pass, since it's not enabled by default.

@gravitational-jenkins
Copy link

Can one of the admins verify this patch?

webvictim
webvictim approved these changes Apr 8, 2020
View reviewed changes
Copy link
Contributor

@webvictim webvictim left a comment

Choose a reason for hiding this comment

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

LGTM from a first look. It'd be good to port this logic to the other Teleport Makefiles as well, as I know many of them have the same hardcoded paths to tests.

@webvictim webvictim removed their assignment Apr 8, 2020
@webvictim
Copy link
Contributor

ok to test

@awly awly force-pushed the improve-make-test branch from 34f9c03 to 9090c1c Compare April 8, 2020 21:16
@awly
Copy link
Contributor Author

awly commented Apr 8, 2020

It'd be good to port this logic to the other Teleport Makefiles as well, as I know many of them have the same hardcoded paths to tests.

I couldn't find other Makefiles in the same repo with hardcoded paths for go test. Could you link a few?

@awly
Copy link
Contributor Author

awly commented Apr 8, 2020

Looks like unit tests pass but integration test fails. Seems to be unrelated to my changes, but please correct me if not.

@russjones
Copy link
Contributor

@awly Failure is due to flakey tests. Should be fixed when #3536 is merged in.

Andrew Lytvynov added 2 commits April 8, 2020 17:38
Fix broken build in example Go client code
1cd2479 
The API of auth package changed in an incompatible way. Fix the usage.
Ensure make test covers all packages
afe814e 
Selectively listing package paths is error-prone. Use `go list` to get
the complete list instead. Filter out integration tests since they are
slower.

Also, enable the race detector by default. Local `make test` runs should
not skip it.
@awly awly force-pushed the improve-make-test branch from 9090c1c to afe814e Compare April 9, 2020 00:38
@awly
Copy link
Contributor Author

awly commented Apr 9, 2020

@gravitational-jenkins retest this please

1 similar comment
@awly
Copy link
Contributor Author

awly commented Apr 9, 2020

@gravitational-jenkins retest this please

@awly awly merged commit a1df635 into gravitational:master Apr 9, 2020
@awly awly deleted the improve-make-test branch April 16, 2020 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@webvictim webvictim webvictim approved these changes

@russjones russjones russjones approved these changes

@klizhentas klizhentas Awaiting requested review from klizhentas

Assignees

@russjones russjones

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@awly @gravitational-jenkins @webvictim @russjones