Replace Hardhat tests with EDR tests in CI

[fvictorio]

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]

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.

[fvictorio]

Thanks for the answer!

Looking into this in more detail, my guess is that hardhat-core-default-solc and hardhat-core-latest-solc can just be removed. Hardhat only uses solc-js in a single place, for the solc/wrapper module, and that it's covered by the hardhat-sample-project job.

We do have some tests that exercise the code that uses the wrapper... but those are only two tests in a pretty big test suite. I'm not sure it's worth it for you to run those in your CI.

With respect to the stack traces tests (the ones that are already run by the ethereum/solidity repo), those use are using the emscripten build right now, so that's also covered. Ideally those tests would also run with a native binary, but that's a different question and not relevant for this repo.

So I'd suggest just removing those two jobs. If that's ok with you, I can send a PR doing it.

[cameel]

Sounds good. I guess these two jobs don't add much value in that case and we can just drop them.

Ideally those tests would also run with a native binary, but that's a different question and not relevant for this repo.

It would not hurt to run with a native binary as well, but IMO running it with one binary is already good enough, at least assuming you only use the compiler via Standard JSON interface. The binaries should behave the same way for all inputs (we do test that) and we're mostly interested in detecting when changes to the language or outputs break things on your side. Running the test suite on our side is really just a canary and not meant to comprehensively cover all platforms.

[fvictorio]

at least assuming you only use the compiler via Standard JSON interface

That's correct, so it should be fine. We are eventually going to move to native binaries mainly because of the performance of the test suite, but it's not urgent.

Thanks for the clarification then. I'll open a PR with this change.