-
Notifications
You must be signed in to change notification settings - Fork 20
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
Retrieve Blockchain
events
#113
Conversation
9849b73
to
491bf3b
Compare
8e2d950
to
b89bdb9
Compare
@m-Peter Can you try updating to the latest Cadence master in this PR and see if that fixes it? |
@turbolent Sure thing, give me a few minutes to set it up 👍 |
@turbolent Yes, works like a charm. Now I get something like: DBG LOG: flow.AccountContractAdded(codeHash: [93, 30, 218, 233, 148, 57, 196, 34, 191, 78, 54, 3, 253, 41, 197, 212, 65, 138, 60, 82, 151, 184, 56, 82, 34, 5, 198, 112, 25, 103, 186, 220], contract: "FUSD", address: 0xf8d6e0586b0a20c7) The type is loaded correctly. |
Awesome! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
@@ -131,6 +132,8 @@ type TestRunner struct { | |||
// the script environment, in order to aggregate and expose | |||
// log messages from test cases and contracts. | |||
logCollection *LogCollectionHook | |||
|
|||
backend *EmulatorBackend |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@m-Peter The backend
here might be a problem similar to #114 (comment). e.g.: Creating two blockchains, and returning events from both.
Would be good to double-check. Maybe add a test case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I know 😅 I was caught off-guard, I didn't think it would be merged today 😇
However, I did check the previous comment. Why do you think that it would be a problem? The backend
is basically a singleton, it gets instantiated every time the import Test
declaration is encountered, without actually calling Test.newEmulatorBlockchain
. And the way the wiring is done in flow-cli
, https://github.com/onflow/flow-cli/blob/master/internal/test/test.go#L120-L125, a new TestRunner
is created per test file. Can you share a test case where a erroneous side-effect can manifest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry, I just saw that you wrote Maybe add a test case?
😅 I will certainly add tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I'm also not certain whether that is actually a problem, but rather a hunch that it may potentially cause problems.
A good example/test-case would be, in the same test-script:
- Create blockchain,
blockchain1
.- Deploy a contract
Foo
to theblockchain1
, that emits an eventFoo.Event
. - Run a script/transaction that triggers the emitting of event
Foo.Event
. - Then get all events from
blockchain1
.
- Deploy a contract
- Create another blockchain,
blockchain2
.- Deploy a contract
Bar
to theblockchain2
, that emits an eventBar.Event
. - Run a script/transaction that triggers the emitting of event
Bar.Event
. - Then get all events from
blockchain2
.
- Deploy a contract
If this works fine, I guess we should be good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perfect, I will add a case like that!
Work towards: onflow/developer-grants#148
Companion PR: onflow/cadence#2505 ✔️
Description
Provides a way to programmatically retrieve emitted events from the
Blockchain
, from within the testing framework, with:master
branchFiles changed
in the Github PR explorer