Skip to content

Run mutation tests in CI for changed files #1364

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

Open
wants to merge 4 commits into
base: forks/osaka
Choose a base branch
from
Open

Run mutation tests in CI for changed files #1364

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
349e909
run mutmut on changed files
souradeep-das Aug 8, 2025
5942527
style fix
souradeep-das Aug 8, 2025
6cdcdaa
fix review comments
souradeep-das Aug 13, 2025
3ff814d
update test run
souradeep-das Aug 18, 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
64 changes: 64 additions & 0 deletions .github/workflows/test.yaml
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mutation tests might take a while to run - so I'm wondering how we should run them - per PR or commit on main

If several files have changes the time to run on CI also might be unrealistic. In which case I was thinking if we should add more limitations on when to run these tests (or patch mutmut to be able to run mutation tests for specific functions (as opposed to files).
Attached a test run on the main comment ^

Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,67 @@ jobs:
- uses: ./.github/actions/setup-env
- name: Run optimized tests
run: tox -e optimized

mutation-test:
runs-on: ubuntu-latest
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
runs-on: ubuntu-latest
runs-on: [self-hosted-ghr, size-xl-x64]

This is the magic sauce that runs the job on a beefier machine.

needs: static
if: github.event_name == 'pull_request'
strategy:
matrix:
fork: [frontier, homestead, tangerine_whistle, spurious_dragon, byzantium, constantinople, istanbul, berlin, london, paris, shanghai, cancun, prague, osaka]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

some forks do not have a direct tests/{fork_name} file - what should we do for them?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is likely going to change with @gurukamath's recent work on refactoring the tests.

Copy link
Contributor

@SamWilsn SamWilsn Aug 8, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

If you generate the matrix from the output of a previous job (i.e. ${{ needs.changed-forks.outputs.changed-forks }} where changed-forks is a preceding job that stores the changed forks), you can cut down own the number of runs plus use ethereum_spec_tools.forks:Hardfork to dynamically generate the list instead of having to hard code it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks - that sounds great!

steps:
- uses: actions/checkout@v4
with:
submodules: recursive
- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: "3.11"
- name: Install project with test dependencies
run: pip install -e '.[test]'
- name: Install mutmut
run: pip install mutmut==3.3.0
- name: Apply mutmut patch
run: |
git clone https://github.com/souradeep-das/mutate-execution-specs.git
Copy link
Contributor Author

Choose a reason for hiding this comment

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

should i copy the patch to this repo instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of patching, you can install a git dependency:

pip install git+https://github.com/souradeep-das/mutmut.git@<commit hash>

I think that would be easier to maintain than a patch file.

On a separate note, would something like this work (and maybe we can upstream)?

diff --git a/mutmut/__main__.py b/mutmut/__main__.py
index 0fb1ad9..15b16ef 100644
--- a/mutmut/__main__.py
+++ b/mutmut/__main__.py
@@ -407,6 +407,8 @@ class PytestRunner(TestRunner):
         if mutmut.config.debug:
             params = ['-vv'] + params
             print('python -m pytest ', ' '.join(params))
+        if mutmut.config.pytest_extra_args:
+            params += mutmut.config.pytest_extra_args
         exit_code = int(pytest.main(params, **kwargs))
         if mutmut.config.debug:
             print('    exit code', exit_code)
@@ -676,6 +678,7 @@ class Config:
     debug: bool
     paths_to_mutate: List[Path]
     tests_dir: List[str] = None
+    pytest_extra_args: List[str] = None
 
     def should_ignore_for_mutation(self, path):
         if not str(path).endswith('.py'):
@@ -756,6 +759,7 @@ def load_config():
             for y in s('paths_to_mutate', [])
         ] or guess_paths_to_mutate(),
         tests_dir=s('tests_dir', []),
+        pytest_extra_args=s('pytest_extra_args', []),
     )

cp mutate-execution-specs/mutmut.patch .
MUTMUT_FILE_TO_PATCH=$(python3 -c "import mutmut, os; print(os.path.join(os.path.dirname(mutmut.__file__), '__main__.py'))")
patch $MUTMUT_FILE_TO_PATCH < mutmut.patch
- name: Find changed files for fork
id: changed
run: |
git fetch origin ${{ github.event.pull_request.base.ref }}
CHANGED_FILES=$(git diff --name-only origin/${{ github.event.pull_request.base.ref }} ${{ github.sha }} | grep "^src/ethereum/${{ matrix.fork }}/" || true)
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need to grep, or can you just do something like git diff ... -- src/ethereum/${{ matrix.fork }}/?

echo "changed_files<<EOF" >> $GITHUB_OUTPUT
echo "$CHANGED_FILES" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
CHANGED_FILES=$(git diff --name-only origin/${{ github.event.pull_request.base.ref }} ${{ github.sha }} | grep "^src/ethereum/${{ matrix.fork }}/" || true)
echo "changed_files<<EOF" >> $GITHUB_OUTPUT
echo "$CHANGED_FILES" >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT
CHANGED_FILES=$()
echo "changed_files<<EOF" >> $GITHUB_OUTPUT
( git diff --name-only origin/${{ github.event.pull_request.base.ref }} ${{ github.sha }} | grep "^src/ethereum/${{ matrix.fork }}/" || true ) >> $GITHUB_OUTPUT
echo "EOF" >> $GITHUB_OUTPUT

Can avoid storing the list in a temporary variable.

- name: Display changed files
run: |
echo "Changed files:"
echo "${{ steps.changed.outputs.changed_files }}"
- name: Skip if no changes for this fork
if: steps.changed.outputs.changed_files == ''
run: echo "No changes for ${{ matrix.fork }}, skipping mutmut."
- name: Configure mutmut for changed files
if: steps.changed.outputs.changed_files != ''
run: |
echo '[tool.mutmut]' > pyproject.toml
FILES=$(echo "${{ steps.changed.outputs.changed_files }}" | awk '{printf "\"%s\", ", $0}' | sed 's/, $//')
echo "paths_to_mutate = [ $FILES ]" >> pyproject.toml
echo "tests_dir = [\"tests/${{ matrix.fork }}/\"]" >> pyproject.toml
echo "debug = true" >> pyproject.toml
Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, this is... unusual.

So first, this might be easier to implement with shell: python instead of bash. tomllib is in the standard library.

Second, we shouldn't need to modify our project configuration at all to get this to work. I think it would make sense to try and get a patch into mutmut that allows passing pytest arguments via the command line (unlike my patch above) or maybe via an environment variable. We can't be the only project with stupidly long running 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.

On a separate note, would something like this work (and maybe we can upstream)?

Is the patch you mentioned above, for any specific cases that we have in mind? or is it just intended to supply in options like test_dir?

I updated mutmut to take in options through the cli - and its on a PR upstream

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also looks like mutmut is not particularly leaning towards enabling the cli options

Copy link
Contributor Author

Choose a reason for hiding this comment

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

boxed/mutmut#422 (comment)

- name: Verify mutmut config
if: steps.changed.outputs.changed_files != ''
run: |
echo "pyproject.toml contents:"
cat pyproject.toml
- name: Copy dependencies for mutation testing
if: steps.changed.outputs.changed_files != ''
run: |
mkdir -p mutants/src
cp -r src/* mutants/src/
- name: Run mutmut
if: steps.changed.outputs.changed_files != ''
run: |
mutmut run
mutmut results