Drop ColonyNetwork external test run on nightly builds

[r0qs]

Fixes #14230 by dropping it haha

[nikola-matic]

There's quite a few more reference to colony in config.yml - should these be removed as well?

[r0qs]

There's quite a few more reference to colony in config.yml - should these be removed as well?

No, we are just dropping it from the nightly runs as far as I know. Not really sure why they were added there in the first place. Maybe because it is the only external test that runs over solcjs binaries?

The job_ems_compile_ext_colony will not be removed. It runs together with all other external tests for every PR but it is set for compilation only (i.e. it doesn't run the ColonyNetwork tests).

But we should consider updating it to use the upstream repo eventually (currently we are using our own fork). I attempted to do that in a branch, but it still breaking a lot of test cases and require more work: 7b22b02. But this can probably wait.

[cameel]

Not really sure why they were added there in the first place. Maybe because it is the only external test that runs over solcjs binaries?

It's because it was taking ~45 min to run all its tests last time I checked. At some point we decided to keep tests that are relatively quick (~15 min) on PRs and run anything that's slower only in the nightly. The compilation was quick so we kept that on PRs but the full run with the whole test suite was moved to nightly.

Colony was the only one that long back then, but I think now we have some that already take longer than 15 min. Maybe we should consider moving them too.

But we should consider updating it to use the upstream repo eventually (currently we are using our own fork). I attempted to do that in a branch, but it still breaking a lot of test cases and require more work: 7b22b02. But this can probably wait.

Yeah, it's been waiting the whole time. Fixing it was tedious and I never had time for it.

We could actually consider just dropping it completely. The the nice thing about it was that it was pretty low maintenance (it was not breaking often like other tests). And that it's the last ext test we have on Truffle so that has a side-effect of making sure that changes in the compiler don't break it accidentally (or our support for it). Now that it broke and that we're disabling the test suite these benefits are gone.

[r0qs]

We could actually consider just dropping it completely. The the nice thing about it was that it was pretty low maintenance (it was not breaking often like other tests). And that it's the last ext test we have on Truffle so that has a side-effect of making sure that changes in the compiler don't break it accidentally (or our support for it). Now that it broke and that we're disabling the test suite these benefits are gone.

Yes, it would be nice to keep it at least in the PRs then. However, our fork of the ColonyNetwork is 969 commits behind the upstream so it is already kind of pointless to keep it in the way it is, even more now that our fork will not work with Shanghai update, requiring some refactoring. This is why I decided to attempt to update the repo, but it doesn't seem sustainable to keep doing it in this way.

I mean, IMHO we should have tests for the main tools that use the compiler, like foundry, hardhat and truffle for the reasons that you just mentioned. But in the way we are doing we end up spending more time working around bugs or dependencies hell in projects that use those tools. We do need to keep some external tests for measuring gas usage and such, but maybe not all of them.

[cameel]

Oh, one more thing. It would be best to have a short comment on the compile job that remains in code that says why we decided not to run tests for Colony. So that anyone looking at it later does not have to guess.

[r0qs]

Oh, I guess I was to late to do the change. It got merged already. Anyway, I can add the comment in another PR.

Also, I just noticed that from time to time (as for example in this PR), the CI is failing with the following: https://app.circleci.com/pipelines/github/ethereum/solidity/29819/workflows/8bf56b71-d3c4-48ca-ab74-4264020b4886/jobs/1324702?invite=true#step-103-17

curl: (22) The requested URL returned error: 403

I suspect that this is due to the fact that we are not doing authenticated request to github in this specific CI job, and consequently hitting the limit of request per hour that Github imposes on unauthenticated requests (see: https://docs.github.com/en/rest/overview/authenticating-to-the-rest-api?apiVersion=2022-11-28). If this is the case, it is easy to fix (just add the Authorization header).

[cameel]

I mean, IMHO we should have tests for the main tools that use the compiler, like foundry, hardhat and truffle for the reasons that you just mentioned.

We actually do have some now. We have general Truffle and Hardhat tests in solc-js with their own test suites and some tiny example projects so even if we drop Colony completely, it's not that bad.

Also, the way things are going, I kinda doubt we'll add any Truffle external tests again. My hunch is that foundry projects will be much easier to maintain due to not being strapped to the moving target called npm. If that turns out to be true, we'll probably be avoiding adding any JS based ones in the future.

I guess I'm more and more inclined to drop it and not worry about Truffle support in the ext test framework at all.

In the meantime, dropping this from the nigthly is fine.