onflow / cadence

Cadence, the resource-oriented smart contract programming language πŸƒβ€β™‚οΈ
https://developers.flow.com/cadence
Apache License 2.0
531 stars 139 forks source link

Contracts, scripts and transactions as first-class citizens in testing framework #1993

Open psiemens opened 1 year ago

psiemens commented 1 year ago

First off, great work on the Cadence testing framework! It's so awesome to see native testing features.

As I was trying it out, I was a bit surprised to find that contracts, scripts and transactions are still passed as strings inside Cadence. It felt odd to me that I had to put Cadence code in a string inside a Cadence program.

I was then thinking: do we need to have the Blockchain construct in order to test Cadence programs?

Would it be possible to have a version like this?

Sorry this feedback is so late. I wish I'd had a chance to dig into the testing framework sooner. Even if none of this makes sense for the first version, wanted to share some ideas for future versions.

// contracts/Foo.cdc

pub contract Foo {

  pub event BarUpdated(bar: String)

  pub var bar: String

  pub fun setBar(_ bar: String) {
    self.bar = bar

    emit BarUpdated(bar: bar)
  }

  init(bar: String) {
    self.bar = bar
  }
}
// transactions/setBar.cdc

import Foo from "./Foo.cdc"

// Side note: could transactions have names?
transaction setBar(bar: String) {
  prepare(signer: AuthAccount) {
    log(signer.address)
  }

  execute {
    Foo.setBar(bar)
  }
}
// scripts/getBar.cdc

import Foo from "./Foo.cdc"

pub fun main(): String {
  return Foo.bar
}
// testFoo.cdc

import Foo from "contracts/Foo.cdc"
import setBar from "transactions/setBar.cdc"
import getBar from "scripts/getBar.cdc"

let accountA: AuthAccount = createAccount()
let accountB: AuthAccount = createAccount()

// This instantiates the Foo contract and attaches it to account A.
//
// Foo behaves as a global singleton. All other references to Foo
// are now instantiated with "apple" and attached to account A.
//
// This avoids the need to do any import mapping between files and account addresses.
//
// Contract calls that depend on `self.account` will panic unless an account has been set
// with `deployTo`.
Foo(bar: "apple").deployTo(accountA)

assert(Foo.bar == "apple")

// Construct the `setBar` transaction, prepare it with account B and execute.
let result = setBar("banana").prepare(accountB).execute()

// The transaction should log account B's address.
assert(result.logs == [accountB.address])

// The transaction should emit a `BarUpdated` event.
assert(result.events == [Foo.BarUpdated(bar: "banana")])

// Calling a script is as simple as executing the function.
let updatedBar = getBar()

// The transaction should mutate the `Foo.bar` field.
assert(updatedBar == "banana")
psiemens commented 1 year ago

Also, I completely understand if what I'm proposing is outside the scope of a testing feature or doesn't make sense due to fundamental constraints in Cadence.

I'm now wondering if this requires the Cadence interpreter to know too much about blockchain-specific features (i.e. account storage). πŸ€”

SupunS commented 1 year ago

Thank you @psiemens for spending time to test the feature and for the feedback! πŸ™

The test-framework provides two approaches for testing:

Integration tests Integration basically allows you to mimic the real-world scenarios - Creating accounts, deploying contracts, submitting transactions/scripts to the chain, etc. For e.g: the blockchain used for testing is an emulator-backed one in this initial version. But it can also be extended in future to use a local-net, and even testnet as the blockchain for testing. Many of the functionality implemented in the current version is to support this.

Also, one reason to allow reading the content from file (as string) and mapping the import paths to addresses is to allow devs to use the existing project as-is without changing and use them for testing.

Unit testing I think this is mostly what you are suggesting. No blockchain. Can directly import contracts and call their functions. e.g: https://github.com/SupunS/cadence-testing/blob/main/sample_1/unit_tests.cdc

For now, the features supported for unit testing are very limited due to some implementation complexities.

But definitely, this unit testing can be improved a lot, and these ideas are great!

SupunS commented 1 year ago

Added these to the improvements list: https://github.com/onflow/cadence/issues/1889

bluesign commented 1 year ago

I made a very similar comment before @psiemens, but your implementation looks even better.

Maybe even better can be using some helper functions (instead of import syntax), instead of import Foo from "contracts/Foo.cdc" maybe var Foo = getContract("contracts/Foo.cdc") for example.

I'm now wondering if this requires the Cadence interpreter to know too much about blockchain-specific features (i.e. account storage).

I think this is good case to make it like as you said, so Cadence can have more access to chain when in normal use too

psiemens commented 1 year ago

The test-framework provides two approaches for testing:

  • Integration tests
  • Unit tests

I guess I'm proposing a combination of the two approaches you described. I think it should be possible to create accounts, deploy contracts and submit transactions directly inside unit tests.

If the unit testing functionality was extended to support the cases I showed above, as a developer, I probably wouldn't use the emulator-backed blockchain inside Cadence. I like that the unit tests allow me to focus on my Cadence code without any overhead.

If I wanted to test across different networks, I think I'd prefer to use an external client library through the Access API so that I can make sure my application is properly connecting to the blockchain, signing transactions, properly decoding output, etc.

Also, one reason to allow reading the content from file (as string) and mapping the import paths to addresses is to allow devs to use the existing project as-is without changing and use them for testing.

Yeah, I think it's important to allow developers to test the same transaction and script files that they use in their production app. The approach I outlined also aims to allow a project to keep its files as-is (transactions/setBar.cdc and scripts/getBar.cdc are loaded from external files, presumably the same ones that would be loaded by an application or wallet).

SupunS commented 1 year ago

Yeah, makes sense.

The current challenge is to support this within the existing cadence syntax/semantics. For e.g: imagine the test script imports Foo and Bar, where Foo imports Baz in one account (say acct1) , and Bar imports Baz from another account (say acct2). The two Baz contracts have the same name but they are two completely different contracts. Now for the test script, to import both Foo and Bar, I would also need to instantiate/deploy both of the Baz contracts first. With first-class support, this causes name conflicts. i.e: I can't import both the Bazs.

This is just a hypothetical example - but it's possible to have such dependencies. Not saying we can't implement this, but just pointing out some edge cases we need to consider when designing the API/semantics.

SupunS commented 3 months ago

Contracts were made first-class in https://github.com/onflow/cadence-tools/pull/210. And thus also scripts can become first class (methods in the same test script) too.