Skip to content

Add ability to run framework tests separately #4538

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.

Sign up for GitHub

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

Merged
jtraglia merged 15 commits into from
Sep 16, 2025
Merged

Add ability to run framework tests separately #4538

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
e8f6f3d
Improvements to the tests
leolara Aug 27, 2025
3321ef2
Update Makefile
leolara Aug 28, 2025
5dcd5a9
Apply suggestion from @danceratopz
leolara Aug 28, 2025
ec3edc0
Add fw=true option to make test
jtraglia Aug 28, 2025
d05890b
Add framework tests step
jtraglia Aug 28, 2025
32b0482
Merge branch 'master' into leolara/improv-tests-framework
jtraglia Aug 29, 2025
ab76fc9
Fix merge mistakes
jtraglia Aug 29, 2025
b599bd1
Rethink solution
jtraglia Aug 29, 2025
4daf576
Update release & nightly workflows
jtraglia Aug 29, 2025
38bb4b6
Fix workflow step name
jtraglia Aug 29, 2025
1874b0c
Fix mistake & remove continue-on-error line
jtraglia Aug 29, 2025
96f9656
Merge branch 'master' into leolara/improv-tests-framework
leolara Sep 11, 2025
e75a3cd
Merge branch 'master' into leolara/improv-tests-framework
leolara Sep 12, 2025
5ba45c7
Fix test
leolara Sep 15, 2025
666a1ef
Merge branch 'master' into leolara/improv-tests-framework
leolara Sep 15, 2025
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
19 changes: 18 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,26 @@ test: pyspec
$(PRESET) \
$(BLS) \
--junitxml=$(TEST_REPORT_DIR)/test_results.xml \
$(CURDIR)/tests/infra \
$(PYSPEC_DIR)/eth2spec

# Run test framework tests.
#
# To run a specific test, append k=<test>, eg:
# make test k=test_verify_kzg_proof
# To run tests with a specific bls library, append bls=<bls>, eg:
# make test bls=arkworks
test_infra: MAYBE_TEST := $(if $(k),-k=$(k))
Comment thread
leolara marked this conversation as resolved.
Outdated
# Disable parallelism which running a specific test.
# Parallelism makes debugging difficult (print doesn't work).
test_infra: MAYBE_PARALLEL := $(if $(k),,-n auto)
test_infra: pyspec
@mkdir -p $(TEST_REPORT_DIR)
@$(PYTHON_VENV) -m pytest \
$(MAYBE_PARALLEL) \
--capture=no \
$(MAYBE_TEST) \
$(CURDIR)/tests/infra

@jtraglia jtraglia Aug 27, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I want to keep these tests bundled together. make test should run all tests.

@jtraglia jtraglia Aug 27, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, after deleting this, can we fix the typo on line 101:

Disable parallelism which running a specific test.

Should be:

Disable parallelism when running a specific test.

@danceratopz danceratopz Aug 28, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I want to keep these tests bundled together. make test should run all tests.

I think going forward, as more framework tests get added, there would be value in allowing separate execution of these tests:

  1. It creates a clearer separation of intent that makes it immediately clear to a new developer that these "framework tests" have an entirely different purpose.
  2. It allows a developer to iterate faster on framework changes that are covered by fw tests (typically fw tests will run faster than the actual tests).
  3. It allows better reporting, locally, particularly in CI; consensus-specs could have two workflows that would run make tests and make test_infra.

For now, until we improve the CI as in 2., we could add this new test_infra target as a dependency to the test target. This already enables 1. and 2. We can improve the CI in a follow-up PR for 3. and potentially remove the test_infra dep from test.

@jtraglia jtraglia Aug 28, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I understand, but there are better ways of doing this. This solution introduces unnecessary complexity for everyone: (1) we'll need to update documentation telling contributors that they need to run both commands; (2) we need to update the CI checks to run both commands; (3) we'll need to update the release action to run these; (4) we need to update the Makefile help section.

There are two other solutions which I would prefer:

  • Ensure infrastructure tests start with def test_infra_* and use make test k=test_infra.
  • Similar to the current style, make the list of files dynamic and use make test infra-only=true.

@leolara leolara Aug 28, 2025

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I added a TODO list.

Additionally to what @danceratopz said I don't think the normal spec test developer will not need to run the infra tests, so another advantage of having them separated.

@jtraglia jtraglia Aug 28, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No that's not what I'm saying. Seriously, let's not add a separate make command for infrastructure tests. Please.

@jtraglia jtraglia Aug 28, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I pushed ec3edc0 which does it how I think it should be done. Just run make test fw=true.

@jtraglia jtraglia Aug 29, 2025

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

After thinking about it some more, I've come to this solution:

  • make test (tests everything)
  • make test component=fw (runs only framework tests)
  • make test component=pyspec (runs only pyspec tests)

Are we okay with this? Does it do what you want it to?


###############################################################################
# Coverage
###############################################################################
Expand Down
Loading