Skip to content

Conversation

@SupunS
Copy link
Member

@SupunS SupunS commented Aug 24, 2022

Work towards onflow/cadence#331

Description

This PR is to integrate the new Cadence testing framework into the CLI.

Command:

flow test /path/to/test_script.cdc

Sample output:

Running tests...

Test results:
- PASS: testImportingContract
- PASS: testSimpleScript
- PASS: testSimpleTransaction
- FAIL: testAssertionFailure
		assertion failed: manually failed the test case
- PASS: testInvokingContractFunction
- PASS: testReadingFile
- FAIL: testDeployingContractWithImports
		panic: [Error Code: 1101] cadence runtime error Execution failed:
			error: cannot deploy invalid contract
			 --> 9df24568a2615ba869f4b00abde9fb17f9b1ebe1d3cdd45d3f191f7d579e415e:4:7

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

@SupunS SupunS added the Feature A new user feature or a new package API label Aug 24, 2022
@devbugging
Copy link
Contributor

@SupunS this is really awesome!!! 🔥 Thank you. Left some comments but nothing major. The one thing that is also lacking is documentation. Make sure to add that as you finish things up.

@SupunS SupunS force-pushed the supun/test-framework branch from b5e4a10 to 050c90f Compare August 25, 2022 14:21
@SupunS SupunS requested a review from turbolent August 25, 2022 20:02
@SupunS SupunS force-pushed the supun/test-framework branch 3 times, most recently from 16ed3fb to 811fa2e Compare September 27, 2022 21:50
@SupunS SupunS force-pushed the supun/test-framework branch from 811fa2e to 2f0b283 Compare September 27, 2022 22:30
@SupunS SupunS marked this pull request as ready for review September 27, 2022 22:37
@SupunS
Copy link
Member Author

SupunS commented Sep 27, 2022

@sideninja I'll open a separate PR to add documentation, so that this PR doesn't get too big.

@SupunS SupunS mentioned this pull request Sep 27, 2022
6 tasks
@devbugging
Copy link
Contributor

The only question left here is, should we have this as top commands or not? why this changed from last time you were working on it is because I believe we will also add linter, so I'm now wondering should those commands be flow test flow lint or flow cadence test flow cadence lint. In some way lint and test are indicating doing that for flow so in some sense that is justifiable to be as a top-level command. Do you have opinion?

@SupunS
Copy link
Member Author

SupunS commented Sep 28, 2022

I'm happy with either way.

I'm slightly leaning towards having the test command at the top-level, because it's not only a cadence-related command, but also involves other aspects of flow (uses flow.json, test cases may actually test all blockchain-related operations, etc.).

But for some of the other tools like lint, and maybe document-generator (in future), grouping them under cadence makes sense, because they are very cadence specific.

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 work!

@devbugging devbugging self-requested a review September 28, 2022 17:10
Copy link
Contributor

@devbugging devbugging left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you! Nice work

@SupunS SupunS self-assigned this Sep 29, 2022
@SupunS SupunS merged commit 0c579ef into master Sep 29, 2022
@SupunS SupunS deleted the supun/test-framework branch September 29, 2022 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature A new user feature or a new package API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants