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

adapt library for cadence1 #225

Merged
merged 8 commits into from
Apr 2, 2024

Conversation

NtTestAlert
Copy link
Contributor

@NtTestAlert NtTestAlert commented Mar 22, 2024

Closes #224

Description

Various changes to support Cadence 1.0 emulator
All tests are passing

I've changed the workflow to use Cadence 1.0 CLI
This will need to be reverted later on


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

@NtTestAlert NtTestAlert requested review from MaxStalker and a team as code owners March 22, 2024 07:14
Copy link

changeset-bot bot commented Mar 22, 2024

⚠️ No Changeset found

Latest commit: ac55d57

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@jribbink
Copy link
Contributor

Hey @NtTestAlert , thanks for the PR! I will review this & try to release it today :)

@NtTestAlert
Copy link
Contributor Author

Hey @NtTestAlert , thanks for the PR! I will review this & try to release it today :)

Hi, there is one more thing to be done, stable-cadence cli adds -c1 suffix (but only in some versions), and a version check in general would be good.
Will add it in soon.

@NtTestAlert
Copy link
Contributor Author

NtTestAlert commented Mar 25, 2024

Added execName param to emulator, defaulting to flow-c1

With additional changes (support for old transactions/scripts) might allow users to mix emulator versions, in order to test migration. But that is out of the scope of this PR

@jribbink
Copy link
Contributor

jribbink commented Mar 27, 2024

I tried getting CI to work, but I'm having issues so I guess I can just run locally to verify for now (I force pushed to roll back some of my failed attempts to fix it)

I just ran the tests and it looks good, but it seems like signer parsing fails for an entitled account reference as a signer (e.g. auth(SaveValue) &Account as oppose to an unauthorized reference &Account), which is generally speaking what users will be using in their transactions.

Are we able to support this? Seems like it might be a blocker for any real usage of the library?

@NtTestAlert
Copy link
Contributor Author

NtTestAlert commented Mar 27, 2024

The tests for flow-js-testing pass, incl those with authorized signers - it doesn't try to parse them before executing AFAIK
However flow-cadut does not properly parse authorized signers as per other PR, added a fix there.

Pipeline failed because it could not parse version due to the upgrade banner, bypassed by adding --output json to "fool" flow-cli

@NtTestAlert
Copy link
Contributor Author

NtTestAlert commented Mar 28, 2024

Still need to adapt the library to work with shorthand imports
(import "ViewResolver")
In my project I have this hooked in jest and with some regexes that expand it to import ViewResolver from "./ViewResolver.cdc"
I'll try to add it to the library today
Added workaround until flow-cadut is updated

@jribbink jribbink changed the base branch from master to feature/stable-cadence April 2, 2024 03:42
Copy link
Contributor

@jribbink jribbink left a comment

Choose a reason for hiding this comment

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

Awesome!! I've brought this up to date with the latest @onflow/flow-cadut@stable-cadence release & will merge this, release -> @onflow/flow-js-testing@stable-cadence. Thank you for your contribution!

@jribbink jribbink merged commit f4780cd into onflow:feature/stable-cadence Apr 2, 2024
1 check failed
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.

Stable Cadence preview release support
2 participants