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

Migrate polybft e2e framework to edge #797

Conversation

vcastellm
Copy link
Contributor

@vcastellm vcastellm commented Oct 13, 2022

Description

Backport the e2e framework + tests from v3 to edge.

Changes include

  • Bugfix (non-breaking change that solves an issue)
  • Hotfix (change that solves an urgent issue, and requires immediate attention)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (change that is not backwards-compatible and/or changes current functionality)

Breaking changes

Please complete this section if any breaking changes have been made, otherwise delete it

Checklist

  • I have assigned this PR to myself
  • I have added at least 1 reviewer
  • I have added the relevant labels
  • I have updated the official documentation
  • I have added sufficient documentation in code

Testing

  • I have tested this code with the official test suite
  • I have tested this code manually

Manual tests

Running in local

@vcastellm vcastellm changed the base branch from develop to feature/EVM_47_Build_block_with_Txs_in_PolyBFT October 13, 2022 14:21
@codecov
Copy link

codecov bot commented Oct 14, 2022

Codecov Report

❗ No coverage uploaded for pull request base (feature/v3-parity@d5ff74c). Click here to learn what that means.
The diff coverage is n/a.

❗ Current head b7b8af3 differs from pull request most recent head 92dcc55. Consider uploading reports for the commit 92dcc55 to get more accurate results

@@                 Coverage Diff                  @@
##             feature/v3-parity     #797   +/-   ##
====================================================
  Coverage                     ?   46.48%           
====================================================
  Files                        ?      157           
  Lines                        ?    20803           
  Branches                     ?        0           
====================================================
  Hits                         ?     9670           
  Misses                       ?    10430           
  Partials                     ?      703           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@vcastellm vcastellm changed the base branch from feature/EVM_47_Build_block_with_Txs_in_PolyBFT to feature/v3-parity October 17, 2022 13:10
@vcastellm vcastellm force-pushed the feature/EVM-78-migrate-poly-bft-e-2-e-framework-to-edge branch from 9dbee94 to 60532bc Compare October 17, 2022 14:03
@vcastellm vcastellm closed this Oct 18, 2022
@vcastellm vcastellm reopened this Oct 18, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Oct 18, 2022
@vcastellm vcastellm self-assigned this Oct 18, 2022
@vcastellm vcastellm added the feature New update to Polygon Edge label Oct 18, 2022
@vcastellm vcastellm marked this pull request as ready for review October 18, 2022 10:14
Copy link
Collaborator

@Stefan-Ethernal Stefan-Ethernal left a comment

Choose a reason for hiding this comment

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

Generally looks good, but not sure if we should strive to consolidate existing edge framework with this one somewhere in the future?

And just not to forget: we need to port bridge test (and bridge e2e testing framework infrastructure).

go.mod Outdated Show resolved Hide resolved
e2e-polybft/README.md Outdated Show resolved Hide resolved
e2e-polybft/README.md Outdated Show resolved Hide resolved
contracts/system.go Outdated Show resolved Hide resolved
e2e-polybft/framework/test-cluster.go Outdated Show resolved Hide resolved
@vcastellm
Copy link
Contributor Author

Generally looks good, but not sure if we should strive to consolidate existing edge framework with this one somewhere in the future?

Yeah, this should be our goal, but it will be out of the scope in this PR, we still have some work to do, but anyway, the existing e2e framework in edge is going to be removed at some point.

And just not to forget: we need to port bridge test (and bridge e2e testing framework infrastructure).

Same here, pending to do.

vcastellm and others added 6 commits October 18, 2022 15:16
Co-authored-by: Stefan Negovanović <[email protected]>
Co-authored-by: Stefan Negovanović <[email protected]>
…' of github.com:0xPolygon/polygon-edge into feature/EVM-78-migrate-poly-bft-e-2-e-framework-to-edge
Copy link
Collaborator

@Stefan-Ethernal Stefan-Ethernal left a comment

Choose a reason for hiding this comment

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

LGTM, although it would be nice to address the some of the issues reported by gosec:

  • [/github/workspace/e2e-polybft/framework/test-cluster.go:129] - G301 (CWE-276): Expect directory permissions to be 0750 or less (Confidence: HIGH, Severity: MEDIUM)
  • [/github/workspace/e2e-polybft/framework/test-cluster.go:100] - G304 (CWE-22): Potential file inclusion via variable (Confidence: HIGH, Severity: MEDIUM)
  • [/github/workspace/e2e-polybft/framework/test-cluster.go:100] - G302 (CWE-276): Expect file permissions to be 0600 or less (Confidence: HIGH, Severity: MEDIUM)...

Copy link
Collaborator

@goran-ethernal goran-ethernal left a comment

Choose a reason for hiding this comment

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

LGTM. As Stefan said, it would be nice to fix those gosec errors. Not sure about these snyk tests, we can not see their execution 😕

@vcastellm
Copy link
Contributor Author

vcastellm commented Oct 19, 2022

Snyk tests are about some outdated dependencies we have in general across the repo, we'll need to address them in following PRs

Copy link
Contributor

@Kourin1996 Kourin1996 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 to me.

command/genesis/genesis.go Outdated Show resolved Hide resolved
e2e-polybft/framework/test-cluster.go Outdated Show resolved Hide resolved
e2e-polybft/framework/test-cluster.go Outdated Show resolved Hide resolved
@vcastellm vcastellm merged commit d862307 into feature/v3-parity Oct 21, 2022
@vcastellm vcastellm deleted the feature/EVM-78-migrate-poly-bft-e-2-e-framework-to-edge branch October 21, 2022 07:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New update to Polygon Edge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants