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

Update Cadence tests to latest API features #192

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

m-Peter
Copy link
Contributor

@m-Peter m-Peter commented Oct 19, 2023

Work towards: onflow/developer-grants#216

Description

Updates the tests to be compatible with the upcoming version of the Cadence testing framework.


For contributor use:

  • 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
  • Update the version in package.json when there is a change in the smart contracts

@m-Peter m-Peter marked this pull request as ready for review October 31, 2023 09:08
Comment on lines +3 to +4
import "NonFungibleToken"
import "ExampleNFT"
Copy link
Member

Choose a reason for hiding this comment

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

I'm confused why changing these to this kind of import doesn't break the go tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe the specific script is not part of a test case? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

I meant for all of them. The go tests expect everything to be in the form of import NonFungibleToken from "NonFungibleToken" and it replaces the placeholder with an actual address, so in this case it would just replace it with import 0x.... Is that valid if there is only one contract at that address?

It is used in the tests here: https://github.com/onflow/flow-nft/blob/master/lib/go/test/nft_test.go#L56

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey @joshuahannan, good point.

I did check how that works, and the end result of the script looks like:

Script:  // This script borrows an NFT from a collection

import 0x01cf0e2f2f715450
import 0xe03daebed8ca0615

pub fun main(address: Address, id: UInt64) {
    let account = getAccount(address)

    let collectionRef = account
        .getCapability(ExampleNFT.CollectionPublicPath)
        .borrow<&{NonFungibleToken.CollectionPublic}>()
        ?? panic("Could not borrow capability from public collection")

    // Borrow a reference to a specific NFT in the collection
    let _ = collectionRef.borrowNFT(id: id)
}

An import of the form import 0xe03daebed8ca0615 means import every contract from that address. However, you can't have the same import for an address twice:

import 0xe03daebed8ca0615
import 0xe03daebed8ca0615

This would throw an error.

Copy link
Member

Choose a reason for hiding this comment

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

okay, that makes sense. So fine for now, but we might need to update it in the future if we decide to have contracts at the same address

@joshuahannan joshuahannan merged commit 0faf904 into onflow:master Nov 2, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants