Skip to content

_getini_ini: "string" type returns list silently instead of raising TypeError #14808

Closed

Description

@EternalRights

Was comparing _getini_toml and _getini_ini after the addini type expression changes and noticed an inconsistency.

_getini_toml checks isinstance(value, str) for "string":

elif type == "string":
    if not isinstance(value, str):
        raise TypeError(...)
    return value

_getini_ini just returns the value as-is:

elif type == "string":
    return value

In [tool.pytest.ini_options], make_scalar keeps TOML arrays as lists instead of converting to str. So a TOML array for a type="string"
option makes _getini_ini return a list -- _getini_toml would raise.

"int" and "float" in _getini_ini already have the isinstance guard.
"string" is the only scalar type missing it.

Activity

  1. RonnyPfannschmidt commented on Jul 31, 2026

    @RonnyPfannschmidt
    Member

    Confirmed, and thanks for the precise read — the code analysis is exactly right. Some research on top of it, because the fix is bigger than the missing isinstance suggests.

    Reproduction

    # conftest.py
    def pytest_addoption(parser):
        parser.addini("mystr", "a string option", type="string")
    [tool.pytest.ini_options]
    mystr = ["a", "b"]     # -> getini("mystr") == ['a', 'b'], type=list, silent
    
    [tool.pytest]
    mystr = ["a", "b"]     # -> TypeError: config option 'mystr' expects a string, got list

    This is not fallout from the addini type expression work

    git blame puts elif type == "string": return value at holger's commit from 2010-10-31. It has never been guarded.

    The int/float guards next to it arrived much later, with d126389 ("Add 'int' and 'float' ini option types", 2025-02) — they were added because those types were new, not as a hardening pass that overlooked string.

    So the behaviour is fifteen years old. What is new is the contrast: _getini_toml validates strictly and only exists since the native TOML work in 9.1. The inconsistency is real, but it is the strict side that is new, not the lax one.

    The blast radius is the problem

    addini(type=None) normalises to "string":

    # src/_pytest/config/argparsing.py
    if type is None:
        ini_type = "string"

    So every ini option registered without an explicit type is a string option. In pytest core that is 13 of 44 addini calls — cache_dir, minversion, console_output_style, junit_family, faulthandler_timeout, max_warnings and friends. For third-party plugins, omitting type= is the norm rather than the exception.

    Adding the guard therefore does not only affect options that explicitly declared type="string". It turns a silently-returned list into a TypeError for every untyped plugin option whenever a user writes a TOML array in [tool.pytest.ini_options] — and some plugins may well accept both shapes today, deliberately or not:

    parser.addini("untyped", "no type given at all")   # no type=
    [tool.pytest.ini_options]
    untyped = ["a", "b"]     # -> ['a', 'b'] today, TypeError after the fix

    That makes this a breaking change wanting a deprecation path (warn first, raise in a later major), not a drive-by guard.

    One thing to rule out while we are here: the fix does not belong in make_scalar in findpaths.py. It keeps lists as lists on purpose, so that args, linelist and paths options get their lists through. The decision has to sit with the consumer that knows the declared type.

    Longer term

    This is a symptom of something I keep running into: pytest has to ingest configuration from ini files (everything is a string), TOML in ini mode (str | list[str]), TOML in native mode (real types), -o overrides (strings again) and the command line, and then reconcile all of that against one declared type. Every pair of those has its own coercion rules, and the edge cases between them are by far the most painful part to keep straight — this issue is one of them.

    I have been experimenting with unifying that in cot.config.ingest, where the ingestion and the type resolution are one pass rather than per-source elif ladders. Nothing to adopt yet, but it is the direction I would like this area to move, and it is worth knowing that a spot fix here is patching one cell of a matrix we do not currently model as a matrix.

    For this issue specifically I would suggest: deprecation warning first for a non-str value on a string-typed option in ini mode, TypeError in pytest 10.


    🤖 Research generated with Claude Code

  2. EternalRights commented on Jul 31, 2026

    @EternalRights
    ContributorAuthor

    Confirmed, and thanks for the precise read — the code analysis is exactly right. Some research on top of it, because the fix is bigger than the missing isinstance suggests.

    Reproduction

    conftest.py

    def pytest_addoption(parser):
    parser.addini("mystr", "a string option", type="string")
    [tool.pytest.ini_options]
    mystr = ["a", "b"] # -> getini("mystr") == ['a', 'b'], type=list, silent

    [tool.pytest]
    mystr = ["a", "b"] # -> TypeError: config option 'mystr' expects a string, got list

    This is not fallout from the addini type expression work

    git blame puts elif type == "string": return value at holger's commit from 2010-10-31. It has never been guarded.

    The int/float guards next to it arrived much later, with d126389308 ("Add 'int' and 'float' ini option types", 2025-02) — they were added because those types were new, not as a hardening pass that overlooked string.

    So the behaviour is fifteen years old. What is new is the contrast: _getini_toml validates strictly and only exists since the native TOML work in 9.1. The inconsistency is real, but it is the strict side that is new, not the lax one.

    The blast radius is the problem

    addini(type=None) normalises to "string":

    src/_pytest/config/argparsing.py

    if type is None:
    ini_type = "string"
    So every ini option registered without an explicit type is a string option. In pytest core that is 13 of 44 addini calls — cache_dir, minversion, console_output_style, junit_family, faulthandler_timeout, max_warnings and friends. For third-party plugins, omitting type= is the norm rather than the exception.

    Adding the guard therefore does not only affect options that explicitly declared type="string". It turns a silently-returned list into a TypeError for every untyped plugin option whenever a user writes a TOML array in [tool.pytest.ini_options] — and some plugins may well accept both shapes today, deliberately or not:

    parser.addini("untyped", "no type given at all") # no type=
    [tool.pytest.ini_options]
    untyped = ["a", "b"] # -> ['a', 'b'] today, TypeError after the fix
    That makes this a breaking change wanting a deprecation path (warn first, raise in a later major), not a drive-by guard.

    One thing to rule out while we are here: the fix does not belong in make_scalar in findpaths.py. It keeps lists as lists on purpose, so that args, linelist and paths options get their lists through. The decision has to sit with the consumer that knows the declared type.

    Longer term

    This is a symptom of something I keep running into: pytest has to ingest configuration from ini files (everything is a string), TOML in ini mode (str | list[str]), TOML in native mode (real types), -o overrides (strings again) and the command line, and then reconcile all of that against one declared type. Every pair of those has its own coercion rules, and the edge cases between them are by far the most painful part to keep straight — this issue is one of them.

    I have been experimenting with unifying that in cot.config.ingest, where the ingestion and the type resolution are one pass rather than per-source elif ladders. Nothing to adopt yet, but it is the direction I would like this area to move, and it is worth knowing that a spot fix here is patching one cell of a matrix we do not currently model as a matrix.

    For this issue specifically I would suggest: deprecation warning first for a non-str value on a string-typed option in ini mode, TypeError in pytest 10.

    🤖 Research generated with Claude Code

    Makes sense — deprecation warning first, TypeError in 10. I'll put a PR together.

  3. added a commit that references this issue on Aug 25, 2026
    3b17fa7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions