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

kvstore test with built-in docker invoke #747

Closed
wants to merge 1 commit into from

Conversation

greg-szabo
Copy link
Member

@greg-szabo greg-szabo commented Dec 16, 2020

Please compare to #748

This is example code how we could expand our testing framework with Rust-native test prerequisites built into build.rs.

Prerequisite:

  • docker daemon installed and running

Benefits:

  • No additional dependencies
  • Runs natively with cargo test if the endpoint is already running
  • Runs natively with cargo test --features docker if the endpoint has to be set up
  • Nice message during build notifying what docker image is going to be run

Disadvantages:

  • The shiplift library used to manage docker is out-of-date and had to be pointed to master.

  • The docker image cannot be shut down automatically at the end of the test run. docker stop kvstore-test has to be run or Tendermint will keep running in the background.

  • Referenced an issue explaining the need for the change

  • Updated all relevant documentation in docs

  • Updated all code comments where relevant

  • Wrote tests

  • Updated CHANGELOG.md

@codecov-io
Copy link

Codecov Report

Merging #747 (5ce35d8) into master (4225d7f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master    #747   +/-   ##
======================================
  Coverage    43.2%   43.2%           
======================================
  Files         203     203           
  Lines       13118   13118           
  Branches     3456    3456           
======================================
  Hits         5673    5673           
  Misses       7087    7087           
  Partials      358     358           

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 4225d7f...5ce35d8. Read the comment docs.

@greg-szabo
Copy link
Member Author

We are dropping this idea in favour of #748 .

@greg-szabo greg-szabo closed this Dec 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants