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

Add more test assertions for EVM events #5416

Merged

Conversation

m-Peter
Copy link
Collaborator

@m-Peter m-Peter commented Feb 20, 2024

We also changed the format of evm.TransactionExecuted.DeployedContractAddress:
from: 99466ed2e37b892a2ee3e9cd55a98b68f5735db2
to: 0x99466ED2E37B892A2Ee3E9CD55a98b68f5735db2
which is the preferred check-summed variant.

@m-Peter m-Peter force-pushed the evm-events-more-test-assertions branch from a0e81fd to bd72a51 Compare February 21, 2024 16:41
@codecov-commenter
Copy link

codecov-commenter commented Feb 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 57.51%. Comparing base (8f4ecc4) to head (df30b02).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5416      +/-   ##
==========================================
+ Coverage   55.98%   57.51%   +1.52%     
==========================================
  Files        1026      856     -170     
  Lines       99902    85152   -14750     
==========================================
- Hits        55930    48973    -6957     
+ Misses      39677    32364    -7313     
+ Partials     4295     3815     -480     
Flag Coverage Δ
unittests 57.51% <100.00%> (+1.52%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@m-Peter m-Peter force-pushed the evm-events-more-test-assertions branch from 48ffcdd to 1f1e53a Compare February 22, 2024 09:46
@m-Peter m-Peter force-pushed the evm-events-more-test-assertions branch from 1f1e53a to b831b03 Compare February 22, 2024 18:04
@@ -143,7 +143,7 @@ func (p *TransactionExecutedPayload) CadenceEvent() (cadence.Event, error) {
cadence.String(p.Result.VMErrorString()),
cadence.NewUInt8(p.Result.TxType),
cadence.NewUInt64(p.Result.GasConsumed),
cadence.String(hex.EncodeToString(p.Result.DeployedContractAddress.Bytes())),
Copy link
Contributor

Choose a reason for hiding this comment

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

is this for 0x prefixing?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not only for that. The format changes:
from: 99466ed2e37b892a2ee3e9cd55a98b68f5735db2
to: 0x99466ED2E37B892A2Ee3E9CD55a98b68f5735db2
which is the preferred check-summed variant.

@ramtinms
Copy link
Contributor

@m-Peter can we merge this one ?

@m-Peter
Copy link
Collaborator Author

m-Peter commented Feb 26, 2024

@ramtinms Yes, this is good to go I think.

@janezpodhostnik janezpodhostnik added this pull request to the merge queue Feb 26, 2024
Merged via the queue into onflow:master with commit 49aabc9 Feb 26, 2024
50 of 51 checks passed
@m-Peter m-Peter deleted the evm-events-more-test-assertions branch February 26, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants