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

Replace Hardhat with EDR in CI #15461

Merged
merged 1 commit into from
Oct 2, 2024

Conversation

fvictorio
Copy link
Contributor

The CI run's a subset of Hardhat tests, but we have migrated those tests to the EDR repo. This is still working because we also kept the old tests in the Hardhat repo, but we want to remove them. Before doing that though, we need to point solc's CI to the EDR repo.

I tested the steps locally with the latest soljson.js built by the CI, and it worked fine, but we won't know for sure until it actually runs in the CI.

Copy link

Thank you for your contribution to the Solidity compiler! A team member will follow up shortly.

If you haven't read our contributing guidelines and our review checklist before, please do it now, this makes the reviewing process and accepting your contribution smoother.

If you have any questions or need our help, feel free to post them in the PR or talk to us directly on the #solidity-dev channel on Matrix.

@cameel cameel force-pushed the replace-hardhat-with-edr-in-ci branch from a1d922c to 62dc597 Compare October 1, 2024 22:57
Copy link
Member

@cameel cameel left a comment

Choose a reason for hiding this comment

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

Thanks, looks good!

Note that we also run these tests in CI for solc-js: https://github.com/ethereum/solc-js/blob/master/.circleci/config.yml. So if you want to drop them, we need to update that as well.

Also, if cancun issues have been resolved, then we should try to re-enable these tests, which were disabled only due to this:

  • # NOTE: test suite disabled due to dependence on a specific version of Hardhat
    # which does not support shanghai EVM.
  • solidity/.circleci/config.yml

    Lines 1922 to 1926 in 4ab3e36

    # TODO: Dropping the external tests below since they are based on old forks and
    # fail after update the default evm version to cancun.
    #- t_ext: *job_native_compile_ext_trident
    #- t_ext: *job_native_compile_ext_chainlink
    #- t_ext: *job_native_compile_ext_bleeps
  • solidity/.circleci/config.yml

    Lines 1945 to 1949 in 4ab3e36

    # TODO: Dropping the external tests below since they are based on old forks and
    # fail after update the default evm version to cancun.
    #- t_native_compile_ext_trident
    #- t_native_compile_ext_chainlink
    #- t_native_compile_ext_bleeps

I'm still approving, since merging the PR even without these is an overall improvement.

@ekpyron ekpyron merged commit 351922c into ethereum:develop Oct 2, 2024
72 checks passed
@ekpyron
Copy link
Member

ekpyron commented Oct 2, 2024

Merged it for now - but we should look indeed also look into the other disabled tests and workarounds and see what we can remove now!

@alcuadrado
Copy link
Member

Thanks for merging this! Franco is going to be OOO for a few weeks. Once he's back, we'll update the other repo and resolve the issues around disabled tests.

In the meantime, we won't delete any test from hardhat's repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants