flow-go icon indicating copy to clipboard operation
flow-go copied to clipboard

[integration test] remove dependency on flow-emulator

Open ramtinms opened this issue 3 years ago • 8 comments

Problem Definition

Currently, we use flow-emulator for some integration testing of the DKG and Cluster QC Voting. flow-emulator naturally is dependent on flow-go, flow-go integration test is dependent on flow-emulator and many 3rd party tools are dependent on both. This has been adding a layer of complexity when doing breaking changes in cadence or flow-go. we need to update both emulator and flow-go at the same time but have a properly released version of master in both. While it's still possible to do the updates it's very time consuming and not trivial.

Proposed Solution

Create a simpler version of flow-emulator for use in testing within flow-go. We can port over the existing functionality we need from flow-emulator. We should simplify the internal version compared to the public version wherever possible.

Internal Emulator features which are needed to replace flow-emulator for these tests:

Definition of Done

  • [ ] update integration tests that are dependent to flow-emulator to still work without using flow-emulator
  • [ ] remove the flow-emulator as a dependency from flow-go/integration

ramtinms avatar Jul 26 '22 16:07 ramtinms

From my perspective it would be very beneficial to continue targeting the flow-emulator for these tests (or a flow-go internal simplified version like Ramtin suggested), if it's possible to do so without these upgrade difficulties:

  • The tests in question are testing a Flow client (engine within a Flow node) against a smart contract deployed on Flow -> the Emulator is specifically created for this use-case.
    • Mocking the Access API would make the tests much less useful, as they would remove any interaction with the smart contract

However, we do have test coverage for these processes at other levels:

  • smart contract unit tests (also targeting the Emulator) in flow-core-contracts (excludes protocol node client software from scope)
  • node software unit tests in flow-go (includes protocol node client software in scope, but excludes smart contracts)
  • epoch integration tests in flow-go (includes protocol node client software and smart contracts in scope, but coarser grained)

Note: Moving flow-go/integration Go module to a separate repo should solve the upgrade challenges, because the problem is a repository-wise circular dependency, not a module-wise circular dependency. Though I don't think we should do that for this reason alone.

Alternatively, we could temporarily remove the replace github.com/onflow/flow-go => ../ statement in integration/go.mod during the upgrade process:

  • Upgrade cadence
  • Upgrade flow-go
    • Update flow-go pin in integration (go get flow-go@master)
    • Remove replace github.com/onflow/flow-go => ../ statement in integration/go.mod
  • Upgrade flow-emulator
  • Upgrade flow-go/integration
    • Replace replace github.com/onflow/flow-go => ../ statement in integration/go.mod

Testing the above flow in https://github.com/onflow/flow-go/pull/3067

Summarizing Options

  1. Modify tests to not depend on flow-emulator

1.a) Mock out blockchain at the client level

undesirable because it avoids testing smart contract execution, which is intended to be in scope for these tests

1.b) Create a flow-go internal Emulator clone

this is the best option in terms of upgrade simplification, requires some resourcing to implement this internal tool

  1. Modify upgrade process to temporarily remove version coupling between flow-go and flow-go/integration

Still significant friction for Cadence team, though less, hopefully can make upgrades simpler in the short term

jordanschalm avatar Aug 23 '22 00:08 jordanschalm

Summarizing thread with @ramtinms and @dsainati1:

  • We agree option 1.b) is the ideal option, but requires some dev time to implement the internal version of Emulator
    • The specific features of the Emulator we will need to port over are enumerated in Proposed Solution in the original comment
  • Making use of Option 2 can make the upgrade process less painful in the interim, but the upgrade process still has friction for the Cadence team until the flow-emulator dependency is removed

jordanschalm avatar Aug 24 '22 23:08 jordanschalm

a friendly ping? We've recently stumbled upon this while working on the FVM refactoring (cc: @pattyshack)

SaveTheRbtz avatar Sep 09 '22 20:09 SaveTheRbtz

Alexey suggested making use of the Emulator as a binary rather than a Go dependency, which is a good idea - some thoughts:

  • Some Emulator features are not exposed via the binary, but we don't use any in these tests 👍
  • We would need to ensure the Emulator binary dependency is kept up to date as FVM/Cadence changes

jordanschalm avatar Sep 12 '22 13:09 jordanschalm

This continues to chew up a lot of cycles managing the cross dependencies, which has gotten more complex with the cadence 1.0 branches. Each Access API update requires 2-3 emulator PRs in addition to the flow-go PR.

A few questions about the existing proposals:

  • Is there a leading contender at this point?
  • Do we have a sense of the amount of effort the different approaches would require?
  • Is it possible to replace the emulator with a regular integration network to remove the dependency entirely?

peterargue avatar Apr 16 '24 19:04 peterargue

Is there a leading contender at this point?

Last we talked, we agreed this is the best option:

1.b) Create a flow-go internal Emulator clone

I don't have a good sense for what's involved in implementing 1b. Hopefully we have something similar to a stripped-down emulator for testing FVM that we can re-purpose.

Is it possible to replace the emulator with a regular integration network to remove the dependency entirely?

I looked into this in more detail and I don't think it will work, for the DKG test in particular. The test uses the emulator to control when new views are entered, which we can't easily do with the integration test network.

jordanschalm avatar Apr 17 '24 13:04 jordanschalm

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

github-actions[bot] avatar Sep 27 '24 02:09 github-actions[bot]

@bluesign has created a PoC of a version of the emulator that lives within flow-go/integration in this branch.

jordanschalm avatar Oct 22 '24 23:10 jordanschalm

Addressed by https://github.com/onflow/flow-go/pull/6592

jordanschalm avatar Nov 21 '24 23:11 jordanschalm