Skip to content

Add tests for bot.converters.#418

Merged
scragly merged 1 commit into
masterfrom
add-converter-tests
Sep 17, 2019
Merged

Add tests for bot.converters.#418
scragly merged 1 commit into
masterfrom
add-converter-tests

Conversation

@jchristgit
Copy link
Copy Markdown
Contributor

I'm going insane. There is a commented out parameter there where pytest fails and I have no idea why. Specifically:

        # ('x' * 128, "Are you insane? That's way too long!"),  # ???ß

Please give that a shot. I have no idea why it's failing, because to the blank eye as well as a Python interpreter (and I directly copied the error string!), the exception raised and the one in the code are the exact same.

@jchristgit jchristgit added a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) type: enhancement a: CI Related to continuous intergration and deployment labels Sep 15, 2019
@jchristgit jchristgit requested a review from scragly September 15, 2019 10:44
@kosayoda
Copy link
Copy Markdown
Contributor

Looks to be a pytest problem, minimal reproducible example is this:

import pytest


def func():
    raise ValueError("?")


@pytest.mark.parametrize(
    ("expected"), (("?"),)
)
def test_func(expected):
    with pytest.raises(ValueError, match=expected):
        func()

I have no idea what the reason is, but escaping the ? character with a backslash in only the parametrize call works and will pass the test case.

@pytest.mark.parametrize(
    ("expected"), (("\?"),)
)

@avayert
Copy link
Copy Markdown
Contributor

avayert commented Sep 15, 2019

in the documentation for pytest.raises it says

match –

if specified, a string containing a regular expression, or a regular expression object, that is tested against the string representation of the exception using re.search

passing the result of re.escape as the expected value should work properly

@kosayoda
Copy link
Copy Markdown
Contributor

Ah, that’s definitely it then, I was dumb.

I checked the source and docs for parametrize instead.

@jchristgit
Copy link
Copy Markdown
Contributor Author

Thanks people

@MarkKoz MarkKoz added type: tests and removed a: CI Related to continuous intergration and deployment type: enhancement labels Sep 16, 2019
@scragly scragly merged commit 709c5b8 into master Sep 17, 2019
@scragly scragly deleted the add-converter-tests branch September 17, 2019 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants