Andrew7234/e2e extension

[Andrew7234]

Task

We'd like to run e2e_regression against multiple block ranges. The first range we'd like to add is the old Damask one.

Implementation details TBD. A possible approach we were brainstorming with @pro-wh in the office yesterday:

• We add a Makefile target for each block range; each individual target runs nexus with different config files. The old e2e_regression target just runs all the new targets.
• all the e2e data files (cache, expected outputs, actual outputs) get reorganized in the filesystem: we add an additional level of hierarchy. So they now live in tests/e2e_regression/<range_name>/{actual,expected,rpc_cache} .
• All the scripts (maybe just run.sh and ensure_consistent_config.sh?) are adapted to the new paths
  • run.sh will also have to have separate test cases (URLs) for each block range. Querying for specific accounts, tokens etc only makes sense in a single given block range. For Damask, we can pull these old queries (and ideally even the expected results, to see if we've introduced regressions since?) from git history.
• CI is adapted to run each Makefile target as a separate step, so they can run in parallel.

This PR

This PR extends the e2e_regression tests to accommodate multiple block ranges. Broadly speaking, it

• Moves the range-specific files of each range into their own subdirectory within tests/e2e_regression. The current block range is stored in tests/e2e_regression/eden.
• (Re)adds the damask block range that was used prior to the Eden upgrade
• Adds makefile targets for each range. Note that each range is tested independently; we do not support ingesting multiple disjoint block ranges. Note: the old makefile targets will now run the e2e_regression tests for each block range sequentially.
• Updates scripts and ci accordingly. Notably, test cases in run.sh are now binned by their target block range.

For reviewers: Going commit by commit might be helpful.

Todo:

• Lots of hardcoding range names (eden/damask), which is explicit/simple but also leads to some bloat in the makefile [x]
• double check that the (re)added damask expected responses match the ones removed in e2e-regression: Use block range in Eden #596
• some of the test cases didn't change between eden/damask and were thus reused; should check them more thoroughly [x]
• add cobalt range?? -- not in the PR

[Andrew7234]

🤨

[mitjat]

Todo: add cobalt range??

If we do that (I think Warren has a good use case; roothash events were quite different in cobalt), it should be a separate PR. It's a nontrivial task to identify a good range and relevant queries for that range. I recently did that for Eden and was surprised at how long it took me.

some of the test cases didn't change between eden/damask and were thus reused

Without looking, my guess is that they query stuff from the genesis. I'll just note that it's possibly still valuable to keep those seemingly duplicated tests because genesis parsing has some code that is specific to each of the three "generations" (eden, damask, cobalt).

[mitjat]

The cause of this weird status is that test-e2e-regression is a merge blocker for this github repo. You can't tell github "all checks have to pass"; you have to instead list the required tests. So when tests change names, it's a minor PITA. I'll disable the requirement right before your PR goes in, and re-enable (with new names) after that.

[mitjat]

Thanks Andy! I just have some minor comments around code organization, overall this looks good! Looking forward to more robust tests :)

[mitjat]

Hm this really is pushing it for duplication :(. Especially with the finicky stuff in this recipe it feels risky that we'll update one recipe but not its near-copy.

I suggest we do one of two things:

• wrap this into a shell script, call it with different args. accept-e2e-regression-eden and accept-e2e-regression-damask can go away then.
• use env variables to parametrize makefile targets. sth like

accept-e2e-regression-single:
    ifndef SUITE
        $(error Env variable SUITE must be defined)
    endif
    rm -rf ./tests/e2e_regression/$(EDEN)/expected
    etc

accept-e2e-regression:
  SUITE=eden make accept-e2e-regression-single
  SUITE=damask make accept-e2e-regression-single

[Andrew7234]

Hmm I'd prefer not to split the logic into multiple files, esp if we end up doing the same for fill-cache and test-e2e.

Makefile doesn't pass the shell vars to the subcommands though. I'll keep poking at it. Edit* it also doesn't pass env vars to sub-make commands.

[mitjat]

Makefile (or rather make) doesn't technically pass env variables to bash; it substitutes them into the command before the command reaches bash. Note that my snippet above has $(EDEN), not ${EDEN}. The former is make syntax for var substitution. Does that work? I have toy examples locally that work but I might be missing something.

[Andrew7234]

Hm I think the main issue I'm seeing rn is that the makefile ifndef needs to be at the top level and not within a recipe. To check whether a variable is defined within a recipe, we'd use the shell scripting, but bash and zsh have different syntax for doing this and having conditional logic based on the shell in a makefile seems to be a cardinal sin. (Side note: Recent versions of bash/zsh both use the [[ -v SUITE ]] syntax, but it isn't supported in the default bash version on macs (3.2))

[Andrew7234]

Well we can just unindent the ifndef block and it works.. though it looks messy:

accept-e2e-regression-suite:
ifndef SUITE
	$(error SUITE is undefined)
endif
	@# Delete all old expected files first, in case any test cases were renamed or removed.
	rm -rf ./tests/e2e_regression/$(SUITE)/expected

We could instead define a separate recipe to check that the env var is set:

check-suite:
ifndef SUITE
	$(error SUITE is undefined)
endif

Edit**: Went with the first option

[mitjat]

Huh I've certainly learned more about Makefiles through this :). Sorry it's such a time-consuming process. Nice solution in the end IMO!

though it looks messy

Fun fact: make is from the 70s. It's amazing it won't die.
The syntax is about as charming as the tan-striped nicotine-stained couches of the 70s ...