Skip to content

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

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 65 additions & 0 deletions .github/workflows/post_release.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
name: Post Release Automation

on:
release:
types: [published]

permissions:
contents: write
pull-requests: write

jobs:
update-version:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
with:
fetch-depth: 0

- name: Configure git user for actions
run: |
git config --global user.email "[email protected]"
git config --global user.name "GitHub Action"

- name: Set environment variables
run: |
echo "NEW_VERSION=${{ github.event.release.tag_name }}" >> $GITHUB_ENV
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 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?

Copy link
Contributor Author

@spencer-tb spencer-tb Feb 21, 2024

Choose a reason for hiding this comment

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

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:

  1. 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.

  2. 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.

  3. 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. :)

Copy link
Member

@marioevz marioevz Feb 21, 2024

Choose a reason for hiding this comment

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

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:
image

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

image

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.

echo "NEW_BRANCH=release-${{ github.event.release.tag_name }}" >> $GITHUB_ENV
echo "COMMIT_MSG=feat(docs|fw): post ${{ github.event.release.tag_name }} release changes" >> $GITHUB_ENV

- name: Bump version in setup configuration
run: |
sed -i "s/^version = .*/version = $NEW_VERSION/" setup.cfg
git add setup.cfg

- name: Update changelog with release details
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
RELEASE_DATE: $(date +'%Y-%m-%d')
run: |
# Replace unreleased link with the new release information
RELEASE_DATE=$(date +'%Y-%m-%d')
ESCAPED_LINK="https:\/\/github.com\/ethereum\/execution-spec-tests\/releases\/tag\/${NEW_VERSION}"
RELEASE_NAME=$(gh release view ${NEW_VERSION} --json body -q '.body' | grep '^#' | head -n 1 | sed 's/^# //')
sed -i "s/^## 🔜 \[Unreleased\].*/## [${NEW_VERSION}](${ESCAPED_LINK}) - ${RELEASE_DATE}: ${RELEASE_NAME}/" docs/CHANGELOG.md

# Append changelog template for the new unreleased section
{ echo ""; tail -n +5 docs/changelog_section_template.md; } > temp_template.md
sed -i '/\*\*Key:\*\*/ r temp_template.md' docs/CHANGELOG.md
rm temp_template.md

git add docs/CHANGELOG.md

- name: Create pull request with relevant changes
uses: peter-evans/create-pull-request@v4
with:
token: ${{ secrets.GITHUB_TOKEN }}
branch: ${{ env.NEW_BRANCH }}
commit-message: ${{ env.COMMIT_MSG }}
title: ${{ env.COMMIT_MSG }}
body: |
## 🗒️ Description
Post ${{ env.NEW_VERSION }} release changes:
- Bump version within `setup.cfg`.
- Update `CHANGELOG.md` and copy in `changelog_section_template.md`.
base: main
2 changes: 2 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ Test fixtures for use by clients are available for each release on the [Github r
- ✨ Improve handling of the argument passed to `solc --evm-version` when compiling Yul code ([#418](https://github.com/ethereum/execution-spec-tests/pull/418)).
- 🐞 Fix `fill -m yul_test` which failed to filter tests that are (dynamically) marked as a yul test ([#418](https://github.com/ethereum/execution-spec-tests/pull/418)).
- 🔀 Helper methods `to_address`, `to_hash` and `to_hash_bytes` have been deprecated in favor of `Address` and `Hash`, which are automatically detected as opcode parameters and pushed to the stack in the resulting bytecode ([#422](https://github.com/ethereum/execution-spec-tests/pull/422)).
- ✨ Add framework versioning CLI, and `_info` section to fixtures ([#443](https://github.com/ethereum/execution-spec-tests/pull/443)).
- ✨ Create post release github actions workflow, to automate changes required after a release is published ([#443](https://github.com/ethereum/execution-spec-tests/pull/443)).

### 🔧 EVM Tools

Expand Down
2 changes: 1 addition & 1 deletion docs/changelog_section_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

The following can be copy-pasted into the `CHANGELOG.md` file for a new release.

## 🔜 [Unreleased]
## 🔜 [Unreleased](https://github.com/ethereum/execution-spec-tests/releases/tag/version) - 2024-xx-xx

### 🧪 Test Cases

Expand Down
5 changes: 3 additions & 2 deletions setup.cfg
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
[metadata]
name = test-filler
name = execution-spec-tests
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
Copy link
Member

Choose a reason for hiding this comment

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

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

url = https://github.com/ethereum/execution-spec-tests
license_files =
LICENSE
Expand Down Expand Up @@ -67,6 +67,7 @@ lint =
flake8>=6.1.0,<7
pep8-naming==0.13.3
fname8>=0.0.3
GitPython>=3.1
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
GitPython>=3.1

This needs to be in

[options]
install_requires=

not in

[options.extras_require]
lint =

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whoops! Nice catch


docs =
cairosvg>=2.7.0,<3 # required for social plugin (material)
Expand Down
2 changes: 2 additions & 0 deletions src/ethereum_test_tools/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
TestInfo,
)
from .spec.blockchain.types import Block, Header
from .utility.helpers import get_framework_version
from .vm import Opcode, OpcodeCallArg, Opcodes

__all__ = (
Expand Down Expand Up @@ -112,5 +113,6 @@
"cost_memory_bytes",
"eip_2028_transaction_data_cost",
"eip_2028_transaction_data_cost",
"get_framework_version",
"transaction_list_root",
)
1 change: 1 addition & 0 deletions src/ethereum_test_tools/spec/__init__.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Test spec definitions and utilities.
"""

from typing import List, Type

from .base.base_test import BaseFixture, BaseTest, TestSpec, verify_post_alloc
Expand Down
3 changes: 3 additions & 0 deletions src/ethereum_test_tools/spec/base/base_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""
Base test class and helper functions for Ethereum state and blockchain tests.
"""

from abc import abstractmethod
from dataclasses import dataclass, field
from itertools import count
Expand All @@ -16,6 +17,7 @@
from ...common.json import JSONEncoder
from ...common.json import field as json_field
from ...reference_spec.reference_spec import ReferenceSpec
from ...utility.helpers import get_framework_version


def verify_transactions(txs: List[Transaction] | None, result) -> List[int]:
Expand Down Expand Up @@ -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()
Copy link
Member

@marioevz marioevz Feb 20, 2024

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, this could definitely be called only once


@abstractmethod
def to_json(self) -> Dict[str, Any]:
Expand Down
7 changes: 7 additions & 0 deletions src/ethereum_test_tools/utility/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
"""
Utility functions used within the EEST framework.
"""

from .helpers import get_framework_version

__all__ = ("get_framework_version",)
18 changes: 18 additions & 0 deletions src/ethereum_test_tools/utility/helpers.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
"""
Utility functions for the EEST framework.
"""

from importlib.metadata import version

from git import InvalidGitRepositoryError, Repo


def get_framework_version() -> str:
"""
Returns the current version of the EEST framework.
"""
try:
local_commit_hash = Repo(search_parent_directories=True).head.commit.hexsha[:7]
except InvalidGitRepositoryError: # required for some framework tests
local_commit_hash = "unknown"
return f"execution-spec-tests v{version('execution-spec-tests')}-{local_commit_hash}"
21 changes: 20 additions & 1 deletion src/pytest_plugins/test_filler/test_filler.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,14 @@
get_closest_fork_with_solc_support,
get_forks_with_solc_support,
)
from ethereum_test_tools import SPEC_TYPES, BaseTest, FixtureCollector, TestInfo, Yul
from ethereum_test_tools import (
SPEC_TYPES,
BaseTest,
FixtureCollector,
TestInfo,
Yul,
get_framework_version,
)
from evm_transition_tool import FixtureFormats, TransitionTool
from pytest_plugins.spec_version_checker.spec_version_checker import EIPSpecTestItem

Expand Down Expand Up @@ -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",
Copy link
Member

Choose a reason for hiding this comment

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

How about just...

Suggested change
"--version-info",
"--version",

🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

action="store_true",
default=False,
help="Displays the versioning information of the framework.",
)


@pytest.hookimpl(tryfirst=True)
def pytest_configure(config):
Expand Down Expand Up @@ -175,6 +190,10 @@ def pytest_configure(config):
f"{Frontier.solc_min_version()}",
returncode=pytest.ExitCode.USAGE_ERROR,
)
if config.getoption("version_info"):
pytest.exit(
f"'Displaying framework version information'\n" f"> {get_framework_version()}\n"
)
Comment on lines +194 to +196
Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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"



@pytest.hookimpl(trylast=True)
Expand Down
9 changes: 9 additions & 0 deletions stubs/git/__init__.pyi
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
from typing import Any

class Repo:
def __init__(self, path: str = ..., search_parent_directories: bool = ...) -> None: ...
@property
def head(self) -> Any: ...

class InvalidGitRepositoryError(Exception):
pass
1 change: 1 addition & 0 deletions whitelist.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ chainid
changelog
chfast
classdict
CLI
cli2
codeAddr
codecopy
Expand Down