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 E2E test support for ERC20/ERC721 #611

Merged
merged 5 commits into from
Mar 18, 2022
Merged

Conversation

awrichar
Copy link
Contributor

Because some token transactions may generate events of multiple types
(such as a transfer triggering approval changes as a side effect), the
transaction type must be stored instead of being inferred from the event.

Signed-off-by: Andrew Richardson <[email protected]>
Resolves hyperledger#564

Signed-off-by: Andrew Richardson <[email protected]>
@awrichar awrichar changed the base branch from erc20 to main March 18, 2022 03:38
@awrichar
Copy link
Contributor Author

I attempted to add ERC20/ERC721 tokens to the E2E matrix for GitHub PRs - will see if it works and runs here. Open to whether this is needed or not (obviously we only want to run a subset of all combinations on every PR).

@nguyer nguyer closed this Mar 18, 2022
@nguyer nguyer reopened this Mar 18, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #611 (6dc62e6) into main (2e464e7) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #611   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          305       305           
  Lines        18196     18266   +70     
=========================================
+ Hits         18196     18266   +70     
Impacted Files Coverage Δ
internal/database/sqlcommon/tokenpool_sql.go 100.00% <100.00%> (ø)
internal/events/token_pool_created.go 100.00% <100.00%> (ø)
internal/tokens/fftokens/fftokens.go 100.00% <100.00%> (ø)
internal/database/sqlcommon/chart_sql.go 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e464e7...6dc62e6. Read the comment docs.

Copy link
Contributor

@nguyer nguyer left a comment

Choose a reason for hiding this comment

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

One minor thing on the GitHub action, but other than that, looks good

@nguyer
Copy link
Contributor

nguyer commented Mar 18, 2022

And there's a failing E2E test 😕

@awrichar
Copy link
Contributor Author

It's weird that Fabric + ERC1155 even executes (and passes?)... does it just ignore the token provider? I know it's definitely not actually using it.

And yea the failures in ERC20 are expected until the dependent PR noted is merged and tagged in the manifest.

@awrichar
Copy link
Contributor Author

Opened hyperledger/firefly-cli#162 to track the fact that -t is silently ignored by Fabric stacks.

@awrichar awrichar merged commit 7f196be into hyperledger:main Mar 18, 2022
@awrichar awrichar deleted the erc20 branch March 18, 2022 15:32
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.

3 participants