Skip to content

Test ethdebug program output against corresponding schema #16009

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

Open
wants to merge 1 commit into
base: eof_source_locations_unoptimized
Choose a base branch
from

Conversation

clonker
Copy link
Member

@clonker clonker commented Apr 16, 2025

  • clones the ethdebug format repository into a tempdir
  • uses that to build a schema repository so that individual fragments of the schema can be used to construct a validator, e.g.,
    validator = jsonschema.Draft202012Validator(
        schema={"$ref": "schema:ethdebug/format/program"},
        registry=ethdebug_schema_repository
    )
  • has a parametrized test that tests (currently) all combinations of requested output in
    • ["evm.bytecode.ethdebug", "evm.deployedBytecode.ethdebug"] and
    • test files ["input_file.json", "input_file_eof.json"]

Due to the EOF test and invalid instructions with negative offsets, this PR depends on #15994 which fixes/implements these things.

The output without these fixes looks like this: https://app.circleci.com/pipelines/github/ethereum/solidity/39379/workflows/982d78f3-9c42-4849-b48f-cd01a82cd4a4/jobs/1822516

@clonker clonker force-pushed the ethdebug_test_schema branch 4 times, most recently from b59db8c to 976bb36 Compare April 16, 2025 13:50
@clonker clonker marked this pull request as draft April 16, 2025 13:58
@clonker clonker force-pushed the ethdebug_test_schema branch 4 times, most recently from dc9001a to dbdbc38 Compare April 17, 2025 07:54
@clonker clonker added the has dependencies The PR depends on other PRs that must be merged first label Apr 17, 2025
@clonker clonker marked this pull request as ready for review April 17, 2025 08:05
@clonker clonker force-pushed the ethdebug_test_schema branch from dbdbc38 to b857048 Compare April 17, 2025 08:20
@clonker clonker changed the base branch from develop to eof_source_locations_unoptimized April 17, 2025 08:21
@clonker clonker requested review from r0qs and aarlt April 17, 2025 08:28
@clonker clonker force-pushed the eof_source_locations_unoptimized branch 3 times, most recently from 3c02df0 to f9a0985 Compare April 23, 2025 06:51
@clonker clonker force-pushed the ethdebug_test_schema branch 2 times, most recently from 345d936 to dbb9928 Compare April 23, 2025 07:16
return dictionary


@pytest.fixture(params=["input_file.json", "input_file_eof.json"])
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be nice to have this already implemented in a way that it will dynamically pick files and test it.. e.g. the test will search for all json files defined in a specific directory and will then verify whether ethdebug output is correct

Copy link
Member Author

Choose a reason for hiding this comment

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

this is already possible by adding more stuff to the params kwarg. depending on what you want to test, you can rglob json files from some directory ahead of test time and add them there to the list

@clonker clonker requested a review from aarlt April 28, 2025 15:44
@clonker clonker force-pushed the eof_source_locations_unoptimized branch 2 times, most recently from ae80060 to 082d7f9 Compare April 29, 2025 14:42
@aarlt
Copy link
Member

aarlt commented Apr 30, 2025

I think it would be awesome to integrate this within our command-line tests. I could imagine that it would be really nice to allow the following: if in a command-line test directory the special file verify-ethdebugis present, the produced schema will be verified, similar to that special file strip-ethdebug to remove the ethdebug data from the output. It would be great if verify-ethdebug and strip-ethdebug could be present at the same time within a single test directory, this might allow the verification of the ethdebug data before the actual removal is done.

@clonker clonker force-pushed the ethdebug_test_schema branch from dbb9928 to 03e6b2f Compare April 30, 2025 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ethdebug has dependencies The PR depends on other PRs that must be merged first testing 🔨
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants