-
Notifications
You must be signed in to change notification settings - Fork 187
feat(plugins,docs): EIP checklist generator plugin #1679
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
Conversation
…d externally covered items
4efb66e to
4e024da
Compare
danceratopz
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice! Huge boost to transparency and quality control. The output is 🔥 (i had to a screenshot to the description 😆).
One dev-ex improvement I would consider adding would be an enum for all the checklist item IDs (and relevant sub-paths). This would require an additional import, but would then allow IDE completion of checklist items as apposed to copy-pasting a string from the markdown. If you like the idea, we can create a follow-up issue.
It's quite impressive how compact the regex approach is! Instead of searching and manipulating markdown, I would have suggested to re-write the .md content in a structured format and used that to generate the template and checklists, e.g.,
# eip_testing_checklist_schema.yaml
metadata:
version: "1.0"
title: "EIP Execution Layer Testing Checklist Template"
categories:
general:
name: "General"
sections:
code_coverage:
name: "Code Coverage"
items:
- id: "general/code_coverage/eels"
description: "Run produced tests against EELS and verify 100% line coverage"
required: true
- id: "general/code_coverage/test_coverage"
description: "Run coverage on test code itself"
required: true
new_opcode:
name: "New Opcode"
sections:
memory_expansion:
name: "Memory Expansion"
items:
- id: "new_opcode/test/mem_exp/zero_bytes_zero_offset"
description: "Zero bytes expansion with zero-offset"
required: trueBut the current approach seems to work well! If we find it too brittle, we can always move to a more structured approach later 🙂
|
Forgot to add above: I just pushed a few commits to address minor issues in the docs with example commands and HTML formatting! |
|
About the enums, this might help the decision as to their value. I think they'd be more usable, but perhaps less readable? Maybe there's a more readable format? I tried with With strings: @pytest.mark.eip_checklist("new_transaction_type/test/intrinsic_validity/gas_limit/exact")
def test_exact_intrinsic_gas(state_test: StateTestFiller):
passWith enums: from pytest_plugins.filler.eip_checklist_ids import ChecklistID
@pytest.mark.eip_checklist(ChecklistID.NEW_TRANSACTION_TYPE__TEST__INTRINSIC_VALIDITY__GAS_LIMIT__EXACT)
def test_exact_intrinsic_gas(state_test: StateTestFiller):
"""Test transaction with exact intrinsic gas limit."""
pass |
|
Could not make the Enum approach work, I will revisit this in the future but merge as is for now. |
🗒️ Description
Creates the
eip_checklistplugin that automatically generates a filled template (from https://eest.ethereum.org/main/writing_tests/checklist_templates/eip_testing_checklist_template/) for each requested EIP.It has two modes:
To run, add marker
pytest.mark.eip_checklistto a test within an EIP folder and then do:Example Output
🔗 Related Issues
✅ Checklist