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

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

Open
psiemens opened this issue Sep 21, 2022 · 7 comments

Comments

@psiemens
Copy link
Contributor

psiemens commented Sep 21, 2022

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?

  • Contracts and transactions are first-class citizens in the main Cadence test program.
  • Contracts become global singletons, can be attached to accounts. This avoids the need to map contract import paths to addresses.
  • Transactions can be constructed and executed directly in the program (maybe even in phases as my example shows).
  • Scripts are just standard function calls.

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 psiemens added Question Further information is requested Feedback labels Sep 21, 2022
@psiemens
Copy link
Contributor Author

psiemens commented Sep 21, 2022

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
Copy link
Member

SupunS commented Sep 21, 2022

Thank you @psiemens for spending time to test the feature and for the feedback! 🙏

The test-framework provides two approaches for testing:

  • Integration tests
  • Unit tests

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.

  • for e.g: Initializing the contract. Have to live within the existing Cadence syntax and provide a way to initialize.
    • Currently, it is allowed to initialize just like a struct, and call method on it directly from the test-case

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

@SupunS
Copy link
Member

SupunS commented Sep 22, 2022

Added these to the improvements list: #1889

@turbolent turbolent removed their assignment Sep 22, 2022
@bluesign
Copy link
Contributor

bluesign commented Sep 22, 2022

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
Copy link
Contributor Author

psiemens commented Sep 22, 2022

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
Copy link
Member

SupunS commented Sep 22, 2022

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
Copy link
Member

SupunS commented Jun 3, 2024

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

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

No branches or pull requests

4 participants