Skip to content

Conversation

@CyberiaResurrection
Copy link
Collaborator

Following on from the work in "Handle -9 PyTest return code", discussion with @Otto-AA and @boxed revealed it would be desirable to handle missing return codes in full generality.

The simplest way I could think of to do that, trying to eliminate side effects, is to convert status_by_exit_code to a defaultdict, with a default value of "suspicious" - one already defined, and handled, elsewhere in the codebase.

Following on from the work in "Handle -9 PyTest return code",
discussion with @Otto-AA and @boxed revealed it would be desirable
to handle missing return codes in full generality.

The simplest way I could think of to _do_ that, trying to eliminate
side effects, is to convert status_by_exit_code to a defaultdict,
with a default value of "suspicious" - one already defined, and
handled, elsewhere in the codebase.
@Otto-AA
Copy link
Collaborator

Otto-AA commented Aug 28, 2025

Simple, but should work fine. You could test it by removing some other exit codes from the dict (eg the codes for killed) and then checking if it outputs suspicious instead of killed with the mutmut browse command.

@Otto-AA
Copy link
Collaborator

Otto-AA commented Aug 28, 2025

You likely also have to make exit_code_to_emoji a defaultdict too

@Otto-AA
Copy link
Collaborator

Otto-AA commented Aug 28, 2025

I think this line should use a direct dict lookup instead of the .get method (or simply exit_code_to_emoji):

print(emoji_by_status.get(status_by_exit_code.get(exit_code), '?'), mutant_name)

I think with the .get() it won't use the defaultdict

@CyberiaResurrection
Copy link
Collaborator Author

You likely also have to make exit_code_to_emoji a defaultdict too

If the defaultdict value for status_by_exit_code wasn't already in exit_code_to_emoji, yes, definitely. As I've gone with "suspicious", which is already in exit_code_to_emoji , I'm not (at a surface level) seeing why it should be a defaultdict.

@CyberiaResurrection
Copy link
Collaborator Author

Simple, but should work fine. You could test it by removing some other exit codes from the dict (eg the codes for killed) and then checking if it outputs suspicious instead of killed with the mutmut browse command.

Yes, it does - removing the exit code status for -9 results in that particular mutant being logged as "suspicious" instead of "segfault".

@CyberiaResurrection
Copy link
Collaborator Author

I think this line should use a direct dict lookup instead of the .get method (or simply exit_code_to_emoji):

print(emoji_by_status.get(status_by_exit_code.get(exit_code), '?'), mutant_name)

I think with the .get() it won't use the defaultdict

According to https://docs.python.org/3/library/collections.html#collections.defaultdict , your reading of get() behaviour is correct.
On other hand, using the defaultdict lookup is a behaviour change above and beyond what I set out to do in this PR.
On third hand, the ? method (not checked - ie, no tests run) when receiving the result of a test run with a hitherto-unseen exit code could come across as misleading, as at least one test has actually run for the mutant in question.

Sod it, after writing that, I'll make the change you suggested.

Further feedback from @Otto-AA pointed out the irrelevance of the
status_by_exit_code get call, now that it is a defaultdict.  Thus,
the lookup is changed to the direct defaultdict lookup.
@CyberiaResurrection CyberiaResurrection marked this pull request as ready for review August 31, 2025 23:25
@Otto-AA
Copy link
Collaborator

Otto-AA commented Sep 1, 2025

We have following definition:

exit_code_to_emoji = {
    exit_code: emoji_by_status[status]
    for exit_code, status in status_by_exit_code.items()
}

This uses all items in status_by_exit_code to define exit_code_to_emoji. However, this does not copy the defaultdict behaviour. exit_code_to_emoji is still a normal dictionary and not a defaultdict, so it will fail when we access a non existing key.

Thus, I would suggest to make it a defaultdict. Though it's interesting that your testing succeeds even though this is not currently implemented yet.

@boxed
Copy link
Owner

boxed commented Sep 1, 2025

Using .get is an alternative to a defaultdict. It won't raise and can be supplied a default value.

The advantages of a normal dict are that it's nicer to write dict literals, and that "new" keys don't take up space as they do if you use a defaultdict. In this case that doesn't matter much.

@CyberiaResurrection
Copy link
Collaborator Author

@boxed , @Otto-AA , any objections to me merging this one?

@boxed
Copy link
Owner

boxed commented Sep 2, 2025

None. Go ahead.

@CyberiaResurrection CyberiaResurrection merged commit 9f672be into boxed:main Sep 2, 2025
5 checks passed
@CyberiaResurrection CyberiaResurrection deleted the HandleMissingReturnCodes branch September 2, 2025 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants