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

Migrating to DroneCI and moving in go-kosu #21

Merged
merged 27 commits into from Apr 29, 2019
Merged

Conversation

@hrharder
Copy link
Member

hrharder commented Apr 27, 2019

Overview

Testing container-based drone CI.

  • The travis checks will probably definitely fail (will eventually remove .travis.yml)
  • The DroneCI (ci.kosu.io) checks should pass
  • All the same tests* are being run
  • Adjustments to kosu-contract test.sh script

Changes

  • Testing DroneCI over travis (hosted at ci.kosu.io)
  • Using ganache forked process instead of nesting docker containers in Drone
    • required adjusting how kosu-system-contracts tests are started (@Freydal does this seem ok?)
    • semi-hacky process management in connect test startup that should probably be improved
    • *commented out the OrderStream.prototype.add() test due to no defined node RPC (yet)
  • Using custom nodejs image hosted at gcr.io/kosu-io/node for the node tests
  • Using custom golang image for golang tests (what do you think @gchaincl ?)
  • The dev-images package could store custom Dockerfiles for any necessary for CI checks (Drone makes this easy).
    • or do should we store them in the packages themselves? this is what I did with go-kosu
  • Changes to connect for TypeScript reasons (mysterious web3 imports)

Notes

  • Eventually I would like to figure out how to avoid committing the ganache-db into the repository.
    • 0x uses a Docker container running a clique-powered single-node network (via geth)
    • See the Dockerfile here
    • Would something like this help us? Their CI config may be useful too.

Status

  • Prefix PR title with [WIP] if necessary (changes not yet made).
  • Add tests to cover changes as needed.
  • Update documentation as needed.
  • Ready for review
@hrharder

This comment has been minimized.

Copy link
Member Author

hrharder commented Apr 27, 2019

And it seems to be passing: https://ci.kosu.io/ParadigmFoundation/kosu-monorepo/32

Let me know what you guys think on Monday, and hopefully all is done correctly!

@hrharder hrharder requested review from Freydal and gchaincl Apr 27, 2019
@hrharder hrharder mentioned this pull request Apr 27, 2019
1 of 5 tasks complete
@hrharder hrharder changed the title [WIP] Refactor/testing drone ci Migrating to DroneCI and moving in all Kosu code Apr 27, 2019
@hrharder hrharder changed the title Migrating to DroneCI and moving in all Kosu code Migrating to DroneCI and moving in go-kosu Apr 27, 2019
@Freydal

This comment has been minimized.

Copy link
Contributor

Freydal commented Apr 27, 2019

@hrharder It took me a second to realize what I needed to look at with all the changes for the system contract tests. I believe they are nearly identical to what they were if i'm not mistaken (they don't use the test rpc at all).

I looked over the test rpc changes and only glaring issue I see is the lack of a .gitignore. One of the benefits of using docker was the docker filesystem recording all the post migration blocks. By ignoring these files we can hide much of the spammy changes and the force add in the migration script will be circumventing the ignore when updates are needed. There should just then be 2 files that are changed during usage that the ignore file won't cover since it they are changes to a tracked file. We may want to find some solution to prevent these from erroneously being committed. It will completely break the testnet.

Some options to cover the file change issue:

  • git co the 2 files on sigint
  • do not store the migration and migrate them all on testnet startup
    • Due to the nature of starting from scratch and deploying with static nonces since the migrations shouldn't change the resulting addresses should be the same. Storing the db was to support having the db corresponding to the addresses in the json.
  • Some cleaner docker config
@hrharder hrharder merged commit e75d181 into master Apr 29, 2019
2 checks passed
2 checks passed
continuous-integration/drone/pr Build is passing
Details
continuous-integration/drone/push Build is passing
Details
@hrharder hrharder deleted the refactor/testing-drone-ci branch Apr 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.