-
Notifications
You must be signed in to change notification settings - Fork 330
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.
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
base: forks/osaka
Are you sure you want to change the base?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -95,3 +95,67 @@ jobs: | |||||||||||||||||
- uses: ./.github/actions/setup-env | ||||||||||||||||||
- name: Run optimized tests | ||||||||||||||||||
run: tox -e optimized | ||||||||||||||||||
|
||||||||||||||||||
mutation-test: | ||||||||||||||||||
runs-on: ubuntu-latest | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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] | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should i copy the patch to this repo instead? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you need to grep, or can you just do something like |
||||||||||||||||||
echo "changed_files<<EOF" >> $GITHUB_OUTPUT | ||||||||||||||||||
echo "$CHANGED_FILES" >> $GITHUB_OUTPUT | ||||||||||||||||||
echo "EOF" >> $GITHUB_OUTPUT | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
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 | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||||||
- 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 |
There was a problem hiding this comment.
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 ^