fix: a non-string value in a TOML setting exits with a traceback - #2263
Merged
Merged
Conversation
Member
|
This is now released as part of coverage 7.16.0. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A non-string value in a TOML setting exits with a traceback instead of the
"Couldn't read config file" message coverage.py uses for every other config
error.
.coveragerc.tomlcontaining[tool.coverage.paths]/source = ["src", 2]:Both exit 1. TOML makes this easy to hit by accident, since it has real integers
and booleans where the ini parser only ever produced strings.
Cause
Three gaps, all in the same shape:
_get_listchecks that the value is a list, then passes each elementstraight to
substitute_variables, which callsre.subon it. A non-stringelement raises
TypeErrorfrom theremodule. This reachesomit,exclude_lines,source, and every other list setting.getfiledoes not type-check at all, sodata_file = 3raises fromos.pathinstead.[paths]is read outside thetry/except ValueErrorinfrom_filethatconverts these into
ConfigError, so even the existing checks escaped astracebacks there.
The fix
Reuse the existing
_check_typehelper for list elements and forgetfile, sothe message matches the ones the other settings already produce, and move the
[paths]loop inside the sametry/except ValueErrorthe rest offrom_fileuses. No new error machinery.
Verification
test_toml_parse_errorsgains 10 parametrized cases, each run against both.coveragerc.tomlandpyproject.toml.Two mutations, because the change has two independent halves:
Reverting the
tomlconfig.pyguards fails 8 of them:Reverting only the
config.pytry/exceptfails the remaining 2:Both are exceptions escaping
pytest.raises(ConfigError), not import orcollection errors.
tests/test_config.pygoes from 88 passed to 98 passed, 6 skipped, 0 failed.ruff format --check,ruff checkandmypyare clean on the changed files.On the rest of the suite: the failure set is byte-identical before and after
(122 failures both ways,
diffof the sorted names is empty). They areenvironmental here, no C extension and no network, and are present on unmodified
main.tests/test_venv.pywas excluded for needing network. Only Python 3.12is available on this machine.
Disclosure: written with AI assistance (Claude Code). I produced the before and after by running the real
coverageCLI, and ran both mutation checks and the before/after failure-set comparison myself.