onflow / flow-go

A fast, secure, and developer-friendly blockchain built to support the next generation of games, apps, and the digital assets that power them.
GNU Affero General Public License v3.0
529 stars 166 forks source link

[integration test] remove dependency on flow-emulator #2863

Open ramtinms opened 1 year ago

ramtinms commented 1 year ago

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

jordanschalm commented 1 year ago

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:

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

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:

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 commented 1 year ago

Summarizing thread with @ramtinms and @dsainati1:

SaveTheRbtz commented 1 year ago

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

jordanschalm commented 1 year ago

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

peterargue commented 1 month ago

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:

jordanschalm commented 1 month ago

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.