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

all: run Docker dependent tests whenever possible #2029

Merged
merged 7 commits into from
May 9, 2019

Conversation

shantuo
Copy link
Contributor

@shantuo shantuo commented May 8, 2019

Fixes #2005

All tests that require Docker container to run will run when either:

  1. Not on Travis.
  2. On Travis Linux environment, where Docker is available.

@shantuo shantuo requested review from eliben and vangent May 8, 2019 22:42
@googlebot googlebot added the cla: yes Google CLA has been signed! label May 8, 2019
@codecov
Copy link

codecov bot commented May 8, 2019

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2029      +/-   ##
==========================================
+ Coverage   77.56%   77.83%   +0.27%     
==========================================
  Files          82       82              
  Lines        9673     9675       +2     
==========================================
+ Hits         7503     7531      +28     
+ Misses       1628     1602      -26     
  Partials      542      542
Impacted Files Coverage Δ
internal/docstore/dynamodocstore/dynamo.go 68.67% <0%> (+0.63%) ⬆️
internal/docstore/mongodocstore/mongo.go 71.24% <0%> (+0.85%) ⬆️
internal/docstore/docstore.go 84.41% <0%> (+1.94%) ⬆️
internal/docstore/firedocstore/fs.go 66.04% <0%> (+2.04%) ⬆️
internal/docstore/memdocstore/mem.go 76.62% <0%> (+3.24%) ⬆️
secrets/vault/vault.go 83.6% <0%> (+6.55%) ⬆️
internal/docstore/driver/driver.go 33.33% <0%> (+33.33%) ⬆️

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 5f089af...2ec5a11. Read the comment docs.

Copy link
Member

@eliben eliben left a comment

Choose a reason for hiding this comment

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

Thanks for doing this!

secrets/vault/vault_test.go Show resolved Hide resolved
internal/testing/setup/setup.go Outdated Show resolved Hide resolved
@eliben
Copy link
Member

eliben commented May 8, 2019

OOC, how do you verify these tests actually ran on Travis, rather than being skipped?

Copy link
Contributor Author

@shantuo shantuo left a comment

Choose a reason for hiding this comment

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

I ran locally with the travis env, our Travis script doesn't have -v for go test so can't tell from there.

secrets/vault/vault_test.go Show resolved Hide resolved
Copy link
Contributor

@vangent vangent left a comment

Choose a reason for hiding this comment

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

Some minor suggestions, otherwise LGTM.

pubsub/kafkapubsub/kafka_test.go Outdated Show resolved Hide resolved
pubsub/kafkapubsub/kafka_test.go Outdated Show resolved Hide resolved
runtimevar/etcdvar/etcdvar_test.go Outdated Show resolved Hide resolved
secrets/vault/vault_test.go Outdated Show resolved Hide resolved
@eliben
Copy link
Member

eliben commented May 9, 2019

I ran locally with the travis env, our Travis script doesn't have -v for go test so can't tell from there.

I think it's worthwhile to test this at least once (like add -v in your PR and remove it later)?

Copy link
Contributor Author

@shantuo shantuo left a comment

Choose a reason for hiding this comment

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

Turning on -v to check the tests are running/skipping correctly and will turn if off later.

pubsub/kafkapubsub/kafka_test.go Outdated Show resolved Hide resolved
pubsub/kafkapubsub/kafka_test.go Outdated Show resolved Hide resolved
runtimevar/etcdvar/etcdvar_test.go Outdated Show resolved Hide resolved
secrets/vault/vault_test.go Outdated Show resolved Hide resolved
@shantuo shantuo merged commit 5915ab4 into google:master May 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Google CLA has been signed!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

testing: explore options to check local container status before running tests
4 participants