Skip to content
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

Improve error message when forced fail test fails #354

Open
wants to merge 35 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
13139ec
Improve error message when forced fail test fails
WillGibson Dec 30, 2024
587f7d3
Update error message with suggestion
WillGibson Dec 30, 2024
70cbf80
Add some sleeps and debug output to rule out race condition for GitHu…
WillGibson Dec 31, 2024
c08280d
Try running on MacOS in case that's the difference
WillGibson Dec 31, 2024
615f592
Revert "Add some sleeps and debug output to rule out race condition f…
WillGibson Dec 31, 2024
2c3113d
Revert "Try running on MacOS in case that's the difference"
WillGibson Dec 31, 2024
85191f2
Merge branch 'main' into improve-failed-forced-fail-error
WillGibson Jan 13, 2025
5c60640
Attempt to see actual output in GitHub Actions
WillGibson Jan 13, 2025
7d7c8b9
More GitHub Actions debug output
WillGibson Jan 13, 2025
4d3050a
Clutching at straws
WillGibson Jan 13, 2025
66b3705
Revert "Clutching at straws"
WillGibson Jan 13, 2025
030faa8
try a sleep in case it is a race condition
WillGibson Jan 13, 2025
ab66e9e
Revert "try a sleep in case it is a race condition"
WillGibson Jan 13, 2025
6fd2e3b
Debug output for GitHub Actions
WillGibson Jan 14, 2025
f94b649
Debug output for GitHub Actions
WillGibson Jan 14, 2025
5777f87
Try explicit catcher.stop()
WillGibson Jan 14, 2025
009f55b
Try with os._exit(1)
WillGibson Jan 14, 2025
712d279
Move the done into the with CatchOutput block
WillGibson Jan 14, 2025
c2f57ab
Try done output in the except block
WillGibson Jan 14, 2025
7faadf9
Try catcher.dump_output()
WillGibson Jan 14, 2025
d5de126
Use SystemExit again
WillGibson Jan 14, 2025
c6ce598
Clutching at straws
WillGibson Jan 14, 2025
134d502
Clutching at straws
WillGibson Jan 14, 2025
bb002bc
sys.stdout.write
WillGibson Jan 15, 2025
de1020d
Revert "sys.stdout.write"
WillGibson Jan 15, 2025
39a748f
Run on macos-15 in GitHub Actions
WillGibson Jan 15, 2025
3084a76
Output some python info
WillGibson Jan 15, 2025
29add96
Revert "Run on macos-15 in GitHub Actions"
WillGibson Jan 18, 2025
713e228
setup-python@v4
WillGibson Jan 18, 2025
8f8c2cd
Drop setup-python@v5
WillGibson Jan 18, 2025
ac19491
Clutching at straws
WillGibson Jan 18, 2025
a7ca7d0
Clutching at straws
WillGibson Jan 18, 2025
43a4bdd
Clutching at straws
WillGibson Jan 18, 2025
a94e680
Clutching at straws
WillGibson Jan 18, 2025
af2663a
Drop CatchOutput
WillGibson Jan 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
8 changes: 4 additions & 4 deletions mutmut/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -885,14 +885,14 @@ def print_stats(source_file_mutation_data_by_path, force_output=False):
print_status(f'{(s.total - s.not_checked)}/{s.total} 🎉 {s.killed} 🫥 {s.no_tests} ⏰ {s.timeout} 🤔 {s.suspicious} 🙁 {s.survived} 🔇 {s.skipped}', force_output=force_output)


def run_forced_fail(runner):
def run_forced_fail_test(runner):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to better differentiate it from the runner's run_force_fail method.

os.environ['MUTANT_UNDER_TEST'] = 'fail'
with CatchOutput(spinner_title='Running forced fail test') as catcher:
try:
if runner.run_forced_fail() == 0:
catcher.dump_output()
print("FAILED")
os._exit(1)
print("Error: Unable to force a test failure during the forced fail test")
raise SystemExit(1)
Copy link
Owner

Choose a reason for hiding this comment

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

I would suggest "FAILED: Unable to force test failures". To be consistent, and for brevity.

Why did you change the exit? I use os._exit() for fast exit, so we don't spend time deallocating at the end of the process.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have update the error message.

I switched to SystemExit because it is much easier to write a test for. Re the performance thing, I think this is something that will only happen once, early on in a run, if something has gone wrong, so some milliseconds taken occasionally doesn't seem like a problem.

Copy link
Contributor Author

@WillGibson WillGibson Dec 30, 2024

Choose a reason for hiding this comment

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

Not sure why test_run_forced_fail_test_with_all_tests_passing is passing locally, but not in GitHub Actions. I'll come back to that when I next have some time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All the unit tests I added are failing in GitHub Actions because the output emitted by run_forced_fail_test is not making it to stdout.

I have tried adding some sleeps to rule out a race condition and changing the runner to MacOS in case that was a factor. No banana.

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 I have poked around and set things up so that there is a test that does not capture the output and running single tests with verbose etc.

If you look at this GitHub Action run, it seems that CatchOutput is only capturing the first line of the output in GitHub Actions for some reason. You can see "done" (and "FAILED" for come reason without the new text) in the test output.

My time for playing this morning has run out. If you have any thoughts on why this might be I would be interested in hearing them.

Copy link
Contributor Author

@WillGibson WillGibson Jan 15, 2025

Choose a reason for hiding this comment

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

Temporarily running the tests on MacOS as well as Ubuntu in GitHub Actions indicates it's not an OS difference.

I'm now wondering whether there is some difference in the way https://github.com/actions/setup-python installs Python compared to how I installed it (via asdf) on my laptop.

@boxed (or anyone else) If you checkout this branch and run the following, does it pass or fail, and how did you install Python?

python -m pytest -vvv --capture=no tests/test_mutation.py::test_run_forced_fail_test_with_failing_test

Copy link
Owner

@boxed boxed Jan 17, 2025

Choose a reason for hiding this comment

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

It passes. I use basic venv and pip.

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. But... damn. I wonder what the difference is in GitHub Actions.

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 I comment out the with CatchOutput part in run_forced_fail_test, capfd works as expected.

I suspect CatchOutput and capfd are doing much the same thing behind the scenes and CatchOutput is usurping capfd. I'll need to read the source code for capfd, but boarding a plane imminently.

@boxed What is the intent/purpose of CatchOutput?

Copy link
Owner

Choose a reason for hiding this comment

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

Yea that makes sense.

The purpose is to not show a lot of output when running mutmut, but being able to dump it on errors. Showing all output to the user would make the tool basically unusable.

except MutmutProgrammaticFailException:
pass
os.environ['MUTANT_UNDER_TEST'] = ''
Expand Down Expand Up @@ -1254,7 +1254,7 @@ def run(mutant_names, *, max_children):
print(' done')

# this can't be the first thing, because it can fail deep inside pytest/django setup and then everything is destroyed
run_forced_fail(runner)
run_forced_fail_test(runner)

runner.prepare_main_test_run()

Expand Down
60 changes: 59 additions & 1 deletion tests/test_mutation.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import os
from io import StringIO
from unittest.mock import Mock

import pytest
from parso import parse

import mutmut
from mutmut.__main__ import (
CLASS_NAME_SEPARATOR,
FuncContext,
Expand All @@ -13,7 +16,7 @@
pragma_no_mutate_lines,
write_all_mutants_to_file,
yield_mutants_for_module,
yield_mutants_for_node,
yield_mutants_for_node, run_forced_fail_test, Config, MutmutProgrammaticFailException,
)


Expand Down Expand Up @@ -407,3 +410,58 @@ def bar():
#
# mutants = mutants_for_source(source)
# assert len(mutants) == 1


def test_run_forced_fail_test_with_failing_test(capfd):
mutmut.config = _default_mutmut_config()
runner = _mocked_runner_run_forced_failed(return_value=1)

run_forced_fail_test(runner)

out, err = capfd.readouterr()
assert 'Running forced fail test' in out
assert 'done' in out
assert os.environ['MUTANT_UNDER_TEST'] is ''


def test_run_forced_fail_test_with_mutmut_programmatic_fail_exception(capfd):
mutmut.config = _default_mutmut_config()
runner = _mocked_runner_run_forced_failed(side_effect=MutmutProgrammaticFailException())

run_forced_fail_test(runner)

out, err = capfd.readouterr()
assert 'Running forced fail test' in out
assert 'done' in out
assert os.environ['MUTANT_UNDER_TEST'] is ''


def test_run_forced_fail_test_with_all_tests_passing(capfd):
mutmut.config = _default_mutmut_config()
runner = _mocked_runner_run_forced_failed(return_value=0)

with pytest.raises(SystemExit) as error:
run_forced_fail_test(runner)

assert error.value.code is 1
out, err = capfd.readouterr()
assert 'Running forced fail test' in out
assert 'Error: Unable to force a test failure during the forced fail test' in out


def _default_mutmut_config():
return Config(
do_not_mutate=[],
also_copy=[],
max_stack_depth=-1,
debug=False,
paths_to_mutate=[]
)

def _mocked_runner_run_forced_failed(return_value=None, side_effect=None):
runner = Mock()
runner.run_forced_fail = Mock(
return_value=return_value,
side_effect=side_effect
)
return runner