-
Notifications
You must be signed in to change notification settings - Fork 973
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Test orientation & distribution format issues #1311
Comments
Survey results:Surveyed most teams, thank you for the quick and extensive responses!
Plan:Split tests into a deeply structured file tree, to filter without memory overhead, at the cost of a bit of disk reading.
The test case formats itself do not change, the properties are just loaded from multiple files, instead of sub-properties of one file. I support the LevelDB idea too, but versioning is important, and we do not get that with leveldb. And instead of zipping the Open questions:
test-viewerI have a POC based on the SSZ collapsible tree-view I implemented for https://simpleserialize.com (awesome site by Chainsafe, check it out). Browse to However, their JS types are not fully usable for 0.8.1 yet, so it cannot load the ssz types that changed yet. Also, there is too much tooling to implement, so I will prioritize testing and the testnetwork live verification tool, Review of attention points
ConfigUnderstand the concern of not changing the config. What I do want to standardize, is how we handle forks in configs. Instead, not many constants change anyway, and forks are considered to be backwards compatible in some basic form (we still need to sync old data, with old code or not). I propose to (minimally) prefix constants that changed in a fork, and prepare prefixes for constants when we know they are going to change. Long term, we can rotate forks out when we do not need to sync it anymore (deprecating it essentially), and remove the prefixes from constants that are considered stable. Prepared change example: Rotation example: Pro: single config, easy config management (no forks), and code can do the switching however it likes. Simple decision, and gets us to transfer testing without complicated configuration or management changes. Sounds good? For forks, the timeline idea still holds: simple key-value mapping to declare the slots for fork names. We can consider other fork activation later, when we have a good example. TLDRDeeper test structure for lower constraints and better filtering. Super small config change to enable forks (and transfer tests without hacks). |
You could commit leveldb files to git? |
@mpetrunic We could, but part of the versioning reasoning is to see which tests are new and/or have been changed. And to roll back a test easily if necessary (as client, or as test maintainer). With raw leveldb chunks you do not get that. As an implementer, you can always write a little script to put the files in leveldb, and use the filepath as key. If that works better for you, please go ahead. |
Let me know what you need |
@GregTheGreek Awesome, thanks. Will implement first things first though (the structure change in test generation). But yes, could use some help in:
If I get to implement the testing change this week, we could get this viewer going some time next week maybe. Let's discuss in discord chat some time. This is the look of the current POC I implemented yesterday (beacon state type not updated, so using the change thing 3x to get a feel for layout): I could put the otherwise static site in a simple firebase hosting wrapper, to deal with routing of dynamic urls. And then we could link to versioned tests by url using the viewing site, instead of the raw ssz on github. |
This issue is set up as a starting point to address inconveniences clients have experienced with the test distribution and/or format.
Pain points
Main pain points identified so far:
1.1) not that nice with CI, new dependency
1.2) clones without LFS active are troublesome
1.3) requires authentication.
2.1) Need to read header to filter for tests, do not want to read the full thing in memory
2.2) Cannot load the full suite of tests in memory
File sizes 0.8.1
As you can see: mainnet files are the biggest source of trouble. Otherwise the maximum individual file size would be 10 MB (SSZ). Or just 2 MB for a single state transition suite. Compare this to the 419 MB block processing test suite for mainnet.
Solutions in status quo
These are the current solutions, not pretty, but functional:
1.1) assumed to run your own docker image in CI already, should be easy to add to the image (even available to Alpine linux)
1.2) Non-LFS clones are just exactly that, we cannot have these large files in the normal Git system, some of these files are just too large to even consider diffing it.
1.3) Authentication shouldn't be required for a public repo. It worked in a CI test setting before. But this is a relatively new issue. And it may be forced due to rate limits / settings. Use the gzipped tar in CI for now instead.
2.1) The first X lines can be read, cut at the
test_cases:
line, and parsed. Not pretty, but the alternative of duplicate data / separate headers is not either. If the files were small, it wouldn't be an issue. (Legacy of early choice for yaml)2.2) Loading it fully in memory before processing is bad, even if parsed into states, the mainnet state objects are still big, too much to keep a few hundred of them in memory. Also consider that there are pre and post states. Too much data to deal with at once really.
Attention points for new solution
Ideas
List of ideas, in no particular order, to think about:
Survey
Please answer in a DM on discord or telegram:
And then I will publicly share anonymized aggregated findings (time TBD). And hopefully find some better solution than status quo.
Please consider answering the following questions (answers may be brief/long):
tests/
in release instead of LFS clone?TLDR
Testing workload and format is a lot to deal with, sharing thoughts + taking survey to make some progress.
The text was updated successfully, but these errors were encountered: