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 tests with EDR tests in CI #752

Open
fvictorio opened this issue Nov 13, 2024 · 1 comment
Open

Replace Hardhat tests with EDR tests in CI #752

fvictorio opened this issue Nov 13, 2024 · 1 comment

Comments

@fvictorio
Copy link

Hi, this issue is meant to be a continuation of ethereum/solidity#15461 As @cameel pointed out in that PR, we also need to update the CI in this repo to be able to remove the network simulation tests from the Hardhat repository which now live in the EDR repository.

I was able manage to adapt the CI in the solidity repo without any issues, but here I think I will need some help/guidance.

From what I can tell, there are three Hardhat-related jobs here:

  • hardhat-core-default-solc
  • hardhat-core-latest-solc
  • hardhat-sample-project

The third one, hardhat-sample-project, I think can be kept as is, since it's more about Hardhat's compilation pipeline than about network simulation features.

The first one, hardhat-core-default-solc, seems a bit weird to me; I'm not sure exactly what it's testing. I don't know if this one can be kept as is or if it can be removed. Adapting it to EDR wouldn't make sense because we don't use npm's solc package there, so the steps here wouldn't do anything.

The second one, hardhat-core-latest-solc, definitely needs to be adapted to run in EDR. For this, I guess I'll need to take the provision-hardhat-with-packaged-solcjs step and create a new one for EDR. I don't foresee any major inconveniences here though.

So I guess I could use an answer about the first job (keep it or remove it), and to know if I'm missing something in this assessment. Thanks a lot!

@cameel
Copy link
Member

cameel commented Nov 22, 2024

The point of hardhat-core-default-solc and hardhat-core-latest-solc was to test your usage of solc-js itself rather than the compiler binary - the CI in the main repo covers that already. We still needed to use some binary though. The most natural thing was to use the binary we package with solc-js, which is what hardhat-core-latest-solc does. For some reason that seemed to run only a small subset of your test suite though. hardhat-core-default-solc was added to also get the whole suite to run by giving Hardhat the solc version that it uses by default, but still with solc-js from the repo. This is a bit awkward though because we need to replace the binary after installing the package.

In any case, if EDR does not use npm's solc, i.e. solc-js, there's not much point testing it here. I think we still want to run Hardhat's test suite here, at least as long as it keeps using solc-js.

I think that for CI here the fact that compiler-related tests were moved to EDR does not really matter. If it was possible to move them in the first place, then it must mean they aren't exercising solc-js anyway, right? Whatever is still left in Hardhat should still be enough to know if our changes here broke it. If that's the case then what we need here is probably to just run the whole test suite rather than the part for hardhat-core. Or, if possible, just some subset that is known to use solc-js.

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

No branches or pull requests

2 participants