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

Strip whitespace from markers in INI config #2867

Merged
merged 1 commit into from
Oct 24, 2017

Conversation

Perlence
Copy link
Contributor

@Perlence Perlence commented Oct 24, 2017

Attempts to resolve #2856.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0008%) to 92.236% when pulling cf2c8e6 on Perlence:ini-markers-whitespace into 7b608f9 on pytest-dev:master.

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

given the simplicity of the solution it seems ok to go for a bugfix

CC @nicoddemus

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

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

Thanks! Please add a CHANGELOG entry and we can merge this. 👍

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0008%) to 92.236% when pulling bbb7635 on Perlence:ini-markers-whitespace into 7b608f9 on pytest-dev:master.

CHANGELOG.rst Outdated
@@ -25,6 +25,9 @@ Bug Fixes
_fixtureinfo attribute (e.g. doctests) (`#2788
<https://github.com/pytest-dev/pytest/issues/2788>`_)

- Strip whitespace from the markers in INI config (`#2856
Copy link
Member

Choose a reason for hiding this comment

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

We use towncrier for changelog management to avoid conflicts, so instead of this please create a file changelog/2856.bugfix with this text:

Strip whitespace from marker names when reading them from INI config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, sorry! I should have paid more attention.

@Perlence Perlence force-pushed the ini-markers-whitespace branch from bbb7635 to 52aadcd Compare October 24, 2017 11:19
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0008%) to 92.236% when pulling 52aadcd on Perlence:ini-markers-whitespace into 7b608f9 on pytest-dev:master.

@bilderbuchi
Copy link
Contributor

For consistency, do you also have to update this section? Although I haven't looked into why this one does not split on (, too.

@Perlence
Copy link
Contributor Author

@bilderbuchi I guess ( is not stripped to show arguments of a mark in CLI.

@nicoddemus nicoddemus merged commit bdab29f into pytest-dev:master Oct 24, 2017
@pquentin
Copy link
Contributor

For what it's worth, this change causes pytest to break in a non obvious way when there's a line without a colon, for example if the description of a marker was wrapped such as:

[pytest]
markers =
    a: A marker description that
       spans multiple lines

@Perlence
Copy link
Contributor Author

@pquentin I reverted the merge, but this doesn't work:

def test_markers_option(testdir):
    testdir.makeini("""
        [pytest]
        markers =
            a1: this is a webtest marker
            a1some: another marker
            a1multi: another marker
                     with stuff
    """)
    result = testdir.runpytest("--markers", )
    result.stdout.fnmatch_lines([
        "*a1*this is a webtest*",
        "*a1some*another marker",
        "*a1multi*another marker*with stuff",
    ])
$ pytest -vvx -k markers_option
...
/home/sviatoslav/workspace/github.com/pytest/testing/test_mark.py:171: Failed
------------------------------------------------------------------------------------------------------------------------------ Captured stdout call ------------------------------------------------------------------------------------------------------------------------------
@pytest.mark.a1: this is a webtest marker

@pytest.mark.a1some: another marker

@pytest.mark.a1multi: another marker

------------------------------------------------------------------------------------------------------------------------------ Captured stderr call ------------------------------------------------------------------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/sviatoslav/workspace/github.com/pytest/_pytest/pytester.py", line 759, in runpytest_inprocess
    reprec = self.inline_run(*args, **kwargs)
  File "/home/sviatoslav/workspace/github.com/pytest/_pytest/pytester.py", line 732, in inline_run
    ret = pytest.main(list(args), plugins=plugins)
  File "/home/sviatoslav/workspace/github.com/pytest/_pytest/config.py", line 58, in main
    return config.hook.pytest_cmdline_main(config=config)
  File "/home/sviatoslav/workspace/github.com/pytest/_pytest/vendored_packages/pluggy.py", line 745, in __call__
    return self._hookexec(self, self._nonwrappers + self._wrappers, kwargs)
  File "/home/sviatoslav/workspace/github.com/pytest/_pytest/vendored_packages/pluggy.py", line 339, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "/home/sviatoslav/workspace/github.com/pytest/_pytest/vendored_packages/pluggy.py", line 334, in <lambda>
    _MultiCall(methods, kwargs, hook.spec_opts).execute()
  File "/home/sviatoslav/workspace/github.com/pytest/_pytest/vendored_packages/pluggy.py", line 614, in execute
    res = hook_impl.function(*args)
  File "/home/sviatoslav/workspace/github.com/pytest/_pytest/mark.py", line 122, in pytest_cmdline_main
    name, rest = line.split(":", 1)
ValueError: not enough values to unpack (expected 2, got 1)

@nicoddemus
Copy link
Member

@pquentin could you please open an issue describing the problem, including the error message you are getting? We can continue this discussion from there then. Thanks!

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.

extra space in INI markers section causes inconsistent behavior
6 participants