Skip to content

Wire up hive tests on CI - #118

Open
SamAg19 wants to merge 9 commits into
grandinetech:devnet-5from
SamAg19:ci-hive
Open

Wire up hive tests on CI#118
SamAg19 wants to merge 9 commits into
grandinetech:devnet-5from
SamAg19:ci-hive

Conversation

@SamAg19

@SamAg19 SamAg19 commented Jul 26, 2026

Copy link
Copy Markdown

fixes #117

Checklist

Self-check your solution before submitting:

  • Verify make test-hive works, on a clean repo checkout, without additional setup (apart from system installation, like go toolchain);
  • Verify that running make test-hive second time doesn't rebuild hive itself;
  • Verify that running make test-hive HIVE_VERSION=<some_other_commit_hash> rebuilds hive properly and re-runs tests;
  • Verify that modifying code in grandine (you can just insert some print in ./lean_client/src/main.rs`) rebuilds docker image and re-runs hive tests;
  • Verify that your script runs all hive tests, available for grandine lean client;
  • Verify your CI works (not necessarily passes, just runs tests) on your own fork;
  • Verify that make clean cleans up hive directory;
  • Don't forget to update README.md - include section what needs to be installed (probably only go toolchain) for running hive tests, and how to run them.

Additions beyond the checklist:

  • Per-suite summary - each suite's pass/total count + names of failed test cases, printed to the job logs and rendered as a GitHub Step Summary panel.
  • Infra-vs-test failure semantics - a completed run with failing tests is reported but does not fail the job; only a genuine build/run failure (no results produced) fails it. A red check means the pipeline broke, not that a spec test failed.
  • Label-gated CI — runs only on PRs with the run-hive label (plus manual workflow_dispatch), not on every push, since the full suite is slow.

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

@SamAg19
SamAg19 marked this pull request as draft July 27, 2026 06:39
@SamAg19
SamAg19 marked this pull request as ready for review July 27, 2026 11:36
@SamAg19

SamAg19 commented Jul 27, 2026

Copy link
Copy Markdown
Author

@ArtiomTr @bomanaps The PR is ready for review

@ArtiomTr ArtiomTr left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Very good, although small fixes still needed

Comment on lines +3 to +5
# 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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

probably fine to run this on every PR, because if it isn't running automatically, then probably no one will run these

Comment thread lean_client/Makefile
Comment on lines +86 to +88
CLIENT_SOURCES := $(shell find . \( -name '*.rs' -o -name 'Cargo.toml' \) \
-not -path './target/*' -not -path './hive/*' -not -path './spec/*' \
-not -path './bin/*') Cargo.lock

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Comment thread lean_client/Makefile
Comment on lines +134 to +138
### 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)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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".

Comment thread lean_client/Makefile
Comment on lines +144 to +148
--client-file simulators/lean/clients/devnet5.yaml \
--client grandine_lean_devnet5 \
--docker.output \
--docker.nocache="hive/clients/grandine_lean_devnet5" \
--results-root ./workspace/logs; \

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not exactly sure, but I think we should be able to parametrize the devnet5 part

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

Successfully merging this pull request may close these issues.

2 participants