feat(fw|ci): add EEST versioning to framework #443

spencer-tb · 2024-02-16T09:49:00Z

🗒️ Description

Adds versioning to the framework. Uses the assumption that the current version of the framework matches that of the latest release. The full version is denoted as: execution-spec-tests v2.1.0-1e4ef51.

A new field is added within the fixtures _info section during filling:

"_info": {
     "framework-version": "execution-spec-tests v2.1.0-1e4ef51"
 }

A CLI flag is added to fill, fill --version-info which outputs the following:

Exit: 'Displaying framework version information'
> execution-spec-tests v2.1.0-1e4ef51c

A new workflow release.yaml is added with jobs to perform when a release is published.
- The versioning is all achieved locally by updating the version field within setup.cfg.
- To avoid us having to update this manually, the workflow currently bumps the version within setup.cfg.
- It additionally updates the changelog, completing jobs we must perform manully after publishing a release.
- The workflow is finalized by creating a PR with the relavant changes to main.

🔗 Related Issues

Resolves: #442
Required for: #436

Remaining Todos

Pre-Review

Consider moving get_framework_version() elsewhere and not within base spec. (Created ethereum_test_tools/utilities)
Consider adding additional jobs to perform within the release workflow, including automatic changelog updates. (Added changelog updates to workflow)

Post-Review

Discuss whether we want seperate versioning for fixtures and test/framework code.
Replace custom versioning solution with existing solution (versioneer).
Discuss optimal method or flow for creating releases.
...

✅ Checklist

All: Set appropriate labels for the changes.
[] All: Considered squashing commits to improve commit history.
All: Added an entry to CHANGELOG.md.
All: Considered updating the online docs in the ./docs/ directory.
Tests: Included the type and version of evm t8n tool used to locally execute test cases: e.g., ref with commit hash or geth 1.13.1-stable-3f40e65.
Tests: Ran mkdocs serve locally and verified the auto-generated docs for new tests in the Test Case Reference are correctly formatted.

spencer-tb · 2024-02-17T04:46:32Z

Requires that settings within https://github.com/ethereum/execution-spec-tests/settings/actions, are set to the following:

spencer-tb · 2024-02-17T14:55:02Z

The workflow should leave us with a PR to main with something like the following: spencer-tb#38

marioevz

Some comments but looks great overall!

marioevz · 2024-02-20T23:31:42Z

src/ethereum_test_tools/spec/base/base_test.py

@@ -100,6 +102,7 @@ def fill_info(
        self.info["filling-transition-tool"] = t8n.version()
        if ref_spec is not None:
            ref_spec.write_info(self.info)
+        self.info["framework-version"] = get_framework_version()


I think we should pass this as a parameter to the filler with type str | None, and ignore if None, but the main advantage would be avoid calling get_framework_version every fixture we need to fill info for, and reusing the data for all tests.

Nice, this could definitely be called only once

marioevz · 2024-02-20T23:32:15Z

setup.cfg

 description = Ethereum execution client test authoring framework
 long_description = file: README.md
 long_description_content_type = text/markdown
-version = 1.0.0
+version = v2.1.0


Oh man, we had not updated this in a long time.

marioevz · 2024-02-20T23:39:06Z

.github/workflows/post_release.yaml

+
+      - name: Set environment variables
+        run: |
+          echo "NEW_VERSION=${{ github.event.release.tag_name }}" >> $GITHUB_ENV


I think the problem is probably going to be that the tag points to a commit that is parent to the commit that the created PR is trying to merge, or I might be misunderstanding something.

Maybe if we do instead a manually triggered workflow that takes the new version number as parameter and does everything, even creating the tag, automatically?

I thought on this a bit. This is true, these changes would create a PR to main after the release. So they would not be included in the release - I am not sure what was going through my mind here. Ideally we do this before the release, so everything is included before we publish.

Following your comment:

Maybe if we do instead a manually triggered workflow that takes the new version number as parameter and does everything, even creating the tag, automatically?

I am thinking we first rename/move CHANGELOG.md to RELEASE_NOTES.md, this would now hold all the release notes and changes for every release so far. We then useCHANGELOG.md as the current log and release notes for what is being worked on. So in a workflow we can simply copy the contents of the changelog to the full set of release notes.

The release process could then be:

We update the new changelog file with everything we want to include in the new release notes, including the description. This would be the final manual PR before drafting a new release, where it allows us all a way to review what the release will look like. This PR would be labelled with something like release-minor, release-major or release-patch, such that it can be used to trigger a workflow when merged.

When this PR is merged it would start the pre_release.yaml workflow. This would essentially automate all the relavant changes that have to occur before a release. First the changelog would be copied into the cumulative release notes file, and the changelog template copied into the changelog file. Secondly the version would be automatically bumped within setup.cfg based on major/minor/patch label we applied. This would then create a PR to main (as we are already doing here) allowing us to review these changes quickly.

When the automatedpre_release.yaml PR is merged to main, this would then trigger the draft_release.yaml workflow. Essentially doing what you suggested. Extract the new version, create/push the new tag with the version, and draft the new release copying the contents within the release notes. We can then review/edit the draft and publish.

What do you think? It should be pretty easy for me to add now - I would add this release flow to the documentation too so its extremely simple to follow. :)

I don't know how easy it would be to detect the PR name but it sounds a bit more complicated from what I had in mind.

With manually trigger I meant to go to https://github.com/ethereum/execution-spec-tests/actions, and under "Workflows", like in this image:

We can create a new workflow that is called "Prepare Release and Tag", which takes the version number here:

Does the automatic commit with all the appropriate changes, and pushes to main (which could be dangerous but we can bail on any minimal error) and then creates the tag that points to the commit that was just pushed.

danceratopz

Really like this! There's two things I'd like to discuss before going into details!

This suggestion tightly couples the execution-spec-tests package version with the fixture release version (i.e. they have the same version 🙂 ). This is probably ok, but it's worth briefly discussing whether they should be distinct. This is only relevant if developers start using execution-spec-tests in their packages. Then it could be worth using two semvers: one for fixtures and one the code/api. For fixtures, we should aim (imo) to reserve a major version bump for changes that require client teams to re-work their CI, a minor bump for new tests and a patch for bug-fixes to a test/fixture release. There would often be an overlap, but often a fixture release wouldn't involve a package release. If we couple these, then downstream users of an execution-spec-tests package will have a hard time making sense of versioning.
Should EEST use a (popular) Python package to help versioning instead of implementing a custom solution? Couple of examples below.
There's a few reasons I'd suggest to go this route.
i. Keep things "more" standard, instead of a completely custom solution, we would follow the best practices used by a common package. There is no single best practice/package in Python as far as I can tell. But typically, for example, a package would get a __version__ property that can be used within the source code.
ii. I think the version string returned below isn't quite sufficient (see comment below). This could be improved without an external package of course. How about something like the following:
- v2.1.1-g1076c97 - if the current head is a tag (in this case v0.1.1)
- v2.1.1+2.g1076c97-dev - if the current head is 2 commits past the last tag (release)
- v2.1.1+2.g1076c97.dirty - if the current repo status is 2 commits past the last tag and dirty (local edits).
Everything before the + is a semver; everything after is to be interpreted as "build metadata".
```
iii. Remove custom github actions code/dependency on Github Actions. Would enable creating a release locally on the command-line (or doing a dry run of creating a release for verification purposes).
```

Examples of versioning packages:

danceratopz · 2024-02-22T09:38:34Z

src/pytest_plugins/test_filler/test_filler.py

@@ -126,6 +133,14 @@ def pytest_addoption(parser):
        help="Path to dump the transition tool debug output.",
    )

+    version_group = parser.getgroup("version_info", "Versioning of the EEST framework")
+    version_group.addoption(
+        "--version-info",


How about just...

Suggested change

"--version-info",

"--version",

🙂

If we use --version it conflicts with the existing flag from pytest

I assumed thats why we used fill --test-help instead of fill --help

I can look into a workaround

danceratopz · 2024-02-22T09:42:09Z

src/pytest_plugins/test_filler/test_filler.py

+        pytest.exit(
+            f"'Displaying framework version information'\n" f"> {get_framework_version()}\n"
+        )


How about?

Suggested change

pytest.exit(

f"'Displaying framework version information'\n" f"> {get_framework_version()}\n"

)

pytest.exit(get_framework_version())

I.e.,

➜ fill --version-info execution-spec-tests v2.1.0-c5a62ea

instead of

➜ fill --version-info Exit: 'Displaying framework version information' > execution-spec-tests v2.1.0-c5a62ea

If we use: pytest.exit(get_framework_version())

We get:

➜ fill --version-info Exit: execution-spec-tests v2.1.0-c5a62ea

I figured it would be nice to have a reason for the exit as there is no way of silencing the "Exit:\n"

danceratopz · 2024-02-22T10:12:08Z

setup.cfg

@@ -67,6 +67,7 @@ lint =
    flake8>=6.1.0,<7
    pep8-naming==0.13.3
    fname8>=0.0.3
+    GitPython>=3.1


Suggested change

GitPython>=3.1

This needs to be in

[options] install_requires=

not in

[options.extras_require] lint =

Whoops! Nice catch

spencer-tb · 2024-02-26T07:58:56Z

2. Should EEST use a (popular) Python package to help versioning instead of implementing a custom solution? Couple of examples below.
   There's a few reasons I'd suggest to go this route.
   i. Keep things "more" standard, instead of a completely custom solution, we would follow the best practices used by a common package. There is no single best practice/package in Python as far as I can tell. But typically, for example, a package would get a `__version__` property that can be used within the source code.
   ii. I think the version string returned below isn't quite sufficient (see comment below). This could be improved without an external package of course. How about something like the following:
   - `v2.1.1-g1076c97` - if the current head is a tag (in this case `v0.1.1`)
   - `v2.1.1+2.g1076c97-dev` - if the current head is 2 commits past the last tag (release)
   - `v2.1.1+2.g1076c97.dirty` - if the current repo status is 2 commits past the last tag and dirty (local edits).
   Everything before the `+` is a semver; everything after is to be interpreted as "build metadata".

Big fan of this!

Versioneer looks ideal for a versioning package :)

I would love to discuss 1) & 3) on Wednesday's call, to determine what the best options are together.

feat(fw|ci): add EEST versioning to framework.

2876795

spencer-tb added scope:ci Scope: Continuous Integration type:feat type: Feature scope:fw Scope: Framework (evm|tools|forks|pytest) labels Feb 16, 2024

spencer-tb added 2 commits February 16, 2024 19:33

feat(fw): small refactor, stubs, tox.

2762842

feat(fw): add ethereum_test_tools/utilities, clean up.

62a7c91

feat(ci): update post release workflow.

f685b90

spencer-tb force-pushed the framework/add-fill-versioning branch from ce5b9c6 to f685b90 Compare February 20, 2024 00:16

feat(docs): changelog entry for EEST versioning and release CI.

c5a62ea

spencer-tb force-pushed the framework/add-fill-versioning branch from 8fefbcf to c5a62ea Compare February 20, 2024 00:38

spencer-tb requested review from marioevz and danceratopz February 20, 2024 00:39

marioevz requested changes Feb 20, 2024

View reviewed changes

danceratopz reviewed Feb 22, 2024

View reviewed changes

This was referenced Feb 26, 2024

feat(fw|ci): add fixture diff utility within CI and local CLI #436

Draft

feat(fw): Add sha256 hash to fixtures #454

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(fw|ci): add EEST versioning to framework #443

feat(fw|ci): add EEST versioning to framework #443

spencer-tb commented Feb 16, 2024 •

edited

spencer-tb commented Feb 17, 2024

spencer-tb commented Feb 17, 2024 •

edited

marioevz left a comment

marioevz Feb 20, 2024 •

edited

spencer-tb Feb 21, 2024

marioevz Feb 20, 2024

marioevz Feb 20, 2024

spencer-tb Feb 21, 2024 •

edited

marioevz Feb 21, 2024 •

edited

danceratopz left a comment

danceratopz Feb 22, 2024

spencer-tb Feb 26, 2024

danceratopz Feb 22, 2024

spencer-tb Feb 26, 2024

danceratopz Feb 22, 2024

spencer-tb Feb 26, 2024

spencer-tb commented Feb 26, 2024

feat(fw|ci): add EEST versioning to framework #443

Are you sure you want to change the base?

feat(fw|ci): add EEST versioning to framework #443

Conversation

spencer-tb commented Feb 16, 2024 • edited

🗒️ Description

🔗 Related Issues

Remaining Todos

Pre-Review

Post-Review

✅ Checklist

spencer-tb commented Feb 17, 2024

spencer-tb commented Feb 17, 2024 • edited

marioevz left a comment

Choose a reason for hiding this comment

marioevz Feb 20, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spencer-tb Feb 21, 2024 • edited

Choose a reason for hiding this comment

marioevz Feb 21, 2024 • edited

Choose a reason for hiding this comment

danceratopz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spencer-tb commented Feb 26, 2024

spencer-tb commented Feb 16, 2024 •

edited

spencer-tb commented Feb 17, 2024 •

edited

marioevz Feb 20, 2024 •

edited

spencer-tb Feb 21, 2024 •

edited

marioevz Feb 21, 2024 •

edited