Wire up hive tests on CI - #118
Conversation
ArtiomTr
left a comment
There was a problem hiding this comment.
Very good, although small fixes still needed
| # Runs on manual dispatch, and on pull requests only when they carry the | ||
| # `run-hive` label (added on label, and re-run on subsequent pushes) so the slow | ||
| # suite doesn't run on every PR push. |
There was a problem hiding this comment.
probably fine to run this on every PR, because if it isn't running automatically, then probably no one will run these
| CLIENT_SOURCES := $(shell find . \( -name '*.rs' -o -name 'Cargo.toml' \) \ | ||
| -not -path './target/*' -not -path './hive/*' -not -path './spec/*' \ | ||
| -not -path './bin/*') Cargo.lock |
There was a problem hiding this comment.
Claude said:
One thing worth knowing: -not -path filters results but doesn't prune traversal, so find still walks all of ./target/, which can be slow on a large build dir. -path './target/*' -prune -o ... -print avoids that.
Don't know if that's true, but probably still worth looking - target directory can indeed be large and slow down source file discovery.
| ### Hive | ||
| # ethereum/hive ref to build. Unset (default) => latest master, resolved at | ||
| # clone time. Override with `make test-hive HIVE_VERSION=<commit>` to pin. | ||
| HIVE_VERSION ?= | ||
| HIVE_REF := $(or $(HIVE_VERSION),master) |
There was a problem hiding this comment.
Probably would be better to put variables on top of the file, next to all other variables - this way it becomes a bit more obvious how we can parametrize our make runs.
There was a problem hiding this comment.
also, probably no need to have 2 variables here - both HIVE_VERSION and HIVE_REF? If you want to set default version, then you can just do:
HIVE_VERSION ?= master?= already means "assign if unset".
| --client-file simulators/lean/clients/devnet5.yaml \ | ||
| --client grandine_lean_devnet5 \ | ||
| --docker.output \ | ||
| --docker.nocache="hive/clients/grandine_lean_devnet5" \ | ||
| --results-root ./workspace/logs; \ |
There was a problem hiding this comment.
Not exactly sure, but I think we should be able to parametrize the devnet5 part
fixes #117
Checklist
Self-check your solution before submitting:
make test-hiveworks, on a clean repo checkout, without additional setup (apart from system installation, likegotoolchain);make test-hivesecond time doesn't rebuild hive itself;make test-hive HIVE_VERSION=<some_other_commit_hash>rebuilds hive properly and re-runs tests;make cleancleans uphivedirectory;Additions beyond the checklist:
Example run (summary panel on the Summary page): https://github.com/SamAg19/lean/actions/runs/30262177584?pr=1; the committed workflow runs the full suite. Example PR where i tested out everything: SamAg19#1