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

[Test] Unify backend environment for Unit & Integration tests #209

Closed
m-Peter opened this issue Sep 10, 2023 · 3 comments · Fixed by #210
Closed

[Test] Unify backend environment for Unit & Integration tests #209

m-Peter opened this issue Sep 10, 2023 · 3 comments · Fixed by #210

Comments

@m-Peter
Copy link
Contributor

m-Peter commented Sep 10, 2023

Issue To Be Solved

Currently, unit testing with the Cadence testing framework has certain shortcomings which make it difficult to use for real-world contracts.

A typical example of a unit test can be seen below:

import Test
import "ArrayUtils"

pub let arrayUtils = ArrayUtils()

pub fun testRange() {
    // Act
    let range = arrayUtils.range(0, 10)

    // Assert
    let expected: [Int] = [0, 1, 2, 3, 4, 5, 6, 7, 8, 9]
    Test.assertEqual(expected, range)
}

The ArrayUtils contract is not deployed on a blockchain, it is simply constructed with the following line:

pub let arrayUtils = ArrayUtils()

The above line is not something that the Language Server can handle, hence, there is no code completion / intellisense for the ArrayUtils contract, regarding its fields / functions / nested types etc.

Under the hood, this is possible because the env.CheckerConfig.ContractValueHandler uses the following:

constructorType, constructorArgumentLabels := sema.CompositeLikeConstructorType(
	checker.Elaboration,
	declaration,
	compositeType,
)

A side-effect of this is that the ArrayUtils contract, can no longer reference itself in nested types:

pub struct Integers {
    pub let numbers: [Int]

    init() {
        self.numbers = ArrayUtils.range(0, 10)
    }
}

Suppose that the ArrayUtils contract had the above nested type. The checker would fail with:

error: Execution failed:
error: value of type `((): ArrayUtils)` has no member `range`
  --> ArrayUtils:73:38
   |
73 |             self.numbers = ArrayUtils.range(0, 10)
   |                                       ^^^^^ unknown member

Another limitation is that the environment in which test scripts are executed, does not support all the operations, e.g.

Execution failed:
error: [Error Code: 1057] operation (GetCurrentBlockHeight) is not supported in this environment
--> ArrayUtils:62:8

Lastly, the ArrayUtils cannot import any other contracts, because nested imports are currently not supported: https://github.com/onflow/cadence-tools/blob/master/test/test_runner.go#L745-L747

(Optional): Suggest A Solution

To overcome all these limitations, we can remove the use of sema.CompositeLikeConstructorType and instead deploy all contracts to a blockchain.

If each TestRunner uses a single EmulatorBackend, then we can easily tap into:

  • env.CheckerConfig.ContractValueHandler
  • env.InterpreterConfig.ImportLocationHandler

This way, we can expose deployed contracts to the test script environment. Developers can choose to access fields / functions / nested types directly. This could be regarded as unit testing. Or they can choose to execute scripts & transactions against the blockchain, which could be regarded as integration testing, since they will be testing their flows end-to-end (pun intended 😇 ).
The EmulatorBackend supports snapshots as well as rolling back to specific block heights, so I believe there is no justified need to create multiple separate blockchains per test file.

The merge of #197, allows developers to reference types from deployed contracts, with:

import Test
import FooContract from 0x0000000000000005

However, imports from AddressLocation is also something that the Language Server cannot handle, which means code completion does not work. By unifying the two environments, we could simply use imports from StringLocation, which would make the code completion work.

@m-Peter
Copy link
Contributor Author

m-Peter commented Sep 10, 2023

After the relevant discussion we had on #197 cc @bluesign @SupunS

@bluesign
Copy link
Contributor

I think testRunner should also utilise flow-cli or flow.json. ( or maybe flow-cli can deploy and run testRunner )

@m-Peter
Copy link
Contributor Author

m-Peter commented Sep 10, 2023

The flow-cli manifests the test execution currently (https://github.com/onflow/flow-cli/blob/master/internal/test/test.go#L128-L134).
It can certainly pass in the contracts key/value from flow.json, in order to remove the need for custom configuration in test scripts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants