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

Cadence testing framework improvements #2696

Merged

Conversation

m-Peter
Copy link
Contributor

@m-Peter m-Peter commented Aug 4, 2023

Closes #2076
Companion PR: onflow/cadence-tools#181

Description

Adding the following improvements:

  • Allow rolling back at a specific block height
  • Add helper function to assert error message on ScriptResult/TransactionResult

Sample usage:

import Test

pub let blockchain = Test.newEmulatorBlockchain()
pub let account = blockchain.createAccount()

// Retrieve current block height
pub let height = getCurrentBlockHeight(blockchain: blockchain)

// Reset (rollback) the blockchain to a given block height
blockchain.reset(height)

pub let script = Test.readFile("../scripts/get_integer_traits.cdc")
pub let result = blockchain.executeScript(script, [])

// Assert that a script/transaction errored and the error message contains a specified sub-string
Test.assertError(result, errorMessage: "exceeding limit")

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@codecov
Copy link

codecov bot commented Aug 4, 2023

Codecov Report

Merging #2696 (ff06b1d) into master (96403f2) will increase coverage by 0.00%.
The diff coverage is 60.00%.

@@           Coverage Diff           @@
##           master    #2696   +/-   ##
=======================================
  Coverage   79.10%   79.10%           
=======================================
  Files         333      333           
  Lines       78192    78194    +2     
=======================================
+ Hits        61851    61855    +4     
+ Misses      14048    14046    -2     
  Partials     2293     2293           
Flag Coverage Δ
unittests 79.10% <60.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
runtime/stdlib/test_emulatorbackend.go 53.01% <60.00%> (+0.61%) ⬆️

@turbolent
Copy link
Member

In general, these additions are great ideas! The FLOW token helpers I'm not so sure about (mintFlow and burnFlow).

Though they are obviously convenient, the Cadence repo is probably not the best place to add them. We're trying to keep Cadence chain-agnostic / independent of Flow. Unfortunately there have been some places where we were not strict (e.g. naming of built-in events), but it would be nice to at least avoid adding new code tying Cadence to Flow, i.e. here the assumption that there is a FLOW token.

Maybe these helper functions could be added upstream, e.g. in cadence-tools/test? That way we would not force embedders of Cadence to provide the FLOW token-specific functionality.

runtime/stdlib/contracts/test.cdc Outdated Show resolved Hide resolved
runtime/stdlib/contracts/test.cdc Outdated Show resolved Hide resolved
Copy link
Contributor

@satyamakgec satyamakgec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

runtime/stdlib/contracts/test.cdc Outdated Show resolved Hide resolved
@m-Peter m-Peter force-pushed the cadence-testing-framework-improvements branch from 7249cb0 to 600ed65 Compare August 7, 2023 08:06
@m-Peter
Copy link
Contributor Author

m-Peter commented Aug 7, 2023

In general, these additions are great ideas! The FLOW token helpers I'm not so sure about (mintFlow and burnFlow).

Though they are obviously convenient, the Cadence repo is probably not the best place to add them. We're trying to keep Cadence chain-agnostic / independent of Flow. Unfortunately there have been some places where we were not strict (e.g. naming of built-in events), but it would be nice to at least avoid adding new code tying Cadence to Flow, i.e. here the assumption that there is a FLOW token.

Maybe these helper functions could be added upstream, e.g. in cadence-tools/test? That way we would not force embedders of Cadence to provide the FLOW token-specific functionality.

@turbolent Thanks for the feedback 🙏 You've got a good point. I wanted to add the helper functions as part of the Test.Blockchain type, but I wasn't satisfied in the first place, because I had to embed the scripts/transactions in Cadence strings. I have moved all 4 helper functions to the companion PR on the cadence-tools/test repository. The new API looks like this:

import Test
import BlockchainHelpers

pub let blockchain = Test.newEmulatorBlockchain()
pub let account = blockchain.createAccount()

// Retrieve current block height
let height = getCurrentBlockHeight(blockchain: blockchain)

// Reset (rollback) the blockchain to a given block height
blockchain.reset(height)

// Retrieve the Flow token balance of a test account
let balance = getFlowBalance(for: account, blockchain: blockchain)

// Mint a given amount of Flow tokens to a specified test account
mintFlow(to: account, amount: 1500.0, blockchain: blockchain)

// Burn a given amount of Flow tokens from a specified test account
burnFlow(from: account, amount: 500.0, blockchain: blockchain)

We have to pass around the blockchain instance, to all these 4 helper functions, as they are no longer defined on the Test.Blockchain type.

@m-Peter m-Peter force-pushed the cadence-testing-framework-improvements branch 3 times, most recently from af7f759 to 01d7af5 Compare August 8, 2023 06:58
@m-Peter m-Peter marked this pull request as ready for review August 8, 2023 07:04
This matcher function asserts that the result of an executed operation,
such as scripts and transactions, resulted in an error that contains
a given sub-string.
@m-Peter m-Peter force-pushed the cadence-testing-framework-improvements branch from 01d7af5 to ff06b1d Compare August 8, 2023 07:36
@m-Peter m-Peter requested a review from turbolent August 8, 2023 09:22
Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@SupunS SupunS merged commit fcb488b into onflow:master Aug 8, 2023
12 of 14 checks passed
@m-Peter m-Peter deleted the cadence-testing-framework-improvements branch August 9, 2023 05:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide detailed information about the error during smart contract execution in test
4 participants