Skip to content
This repository has been archived by the owner on Jun 30, 2021. It is now read-only.

Geth test integration #1127

Merged
merged 11 commits into from
Aug 23, 2019
Merged

Geth test integration #1127

merged 11 commits into from
Aug 23, 2019

Conversation

mederic-p
Copy link
Contributor

@mederic-p mederic-p commented Aug 1, 2019

Issue/Task Number: 1089
Closes #1089

Overview

This PR adds a few integration tests using geth.
Geth must be installed on the machine to successfully run these tests.
Note that I haven't added/changed anything to circle ci pipeline so it won't run these integration tests.

Changes

  • Add a few integration tests.

Implementation Details

A geth node will be started for each integration test using the --dev option.
Each test takes about 7 sec to run.

Usage

The existing tests can be ran as usual with mix test and the new integration tests can be ran using mix test --only integration

@mederic-p mederic-p changed the title Geth test integration [WIP] Geth test integration Aug 1, 2019
@mederic-p mederic-p changed the title [WIP] Geth test integration Geth test integration Aug 6, 2019
@mederic-p mederic-p requested review from sirn, unnawut and T-Dnzt and removed request for sirn August 6, 2019 07:36
@mederic-p mederic-p self-assigned this Aug 6, 2019
@mederic-p mederic-p added this to the v2.0 milestone Aug 6, 2019
Copy link
Contributor

@unnawut unnawut left a comment

Choose a reason for hiding this comment

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

Lovely <3 Some small comments.

apps/eth_blockchain/test/support/dumb_subscriber.ex Outdated Show resolved Hide resolved
apps/eth_blockchain/test/support/dumb_subscriber.ex Outdated Show resolved Hide resolved
apps/eth_blockchain/test/support/wait_for.ex Outdated Show resolved Hide resolved
@unnawut unnawut added this to 4-Review in eWallet via automation Aug 6, 2019
@sirn
Copy link
Contributor

sirn commented Aug 8, 2019

For the CI pipeline, I think the easiest way to do it is to attach Geth as a sidecar in executors.builder_pg (probably rename it to builder_intg or something), e.g. something among the line of:

- image: ethereum/client-go:release-1.9
  entrypoint:
    - geth
    - --dev

Default Geth Dockerfile should expose port 8545, 8546 and 30303 by default, so you can access it via localhost:8545, etc. in the test step. E2E on the other hand rely on docker-compose which you can just add geth as another container in docker-compose.

Copy link

@T-Dnzt T-Dnzt left a comment

Choose a reason for hiding this comment

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

Approving, but probably missing a warning when Geth is already running. If that's the case, the test will silently fail to start Geth and use the currently running one, resulting in unpredictable stuff happening.

@mederic-p mederic-p merged commit fc99e8d into eth-blockchain Aug 23, 2019
eWallet automation moved this from 4-Review to 5-Done Aug 23, 2019
@mederic-p mederic-p deleted the 1089-test-with-geth branch August 23, 2019 03:54
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
eWallet
  
5-Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants