Skip to content

{python3_06} Python3: fix tests init - exceptions and string#216

Merged
anarcat merged 1 commit intolinkchecker:masterfrom
cjmayo:python3_06
Apr 25, 2019
Merged

{python3_06} Python3: fix tests init - exceptions and string#216
anarcat merged 1 commit intolinkchecker:masterfrom
cjmayo:python3_06

Conversation

@cjmayo
Copy link
Copy Markdown
Contributor

@cjmayo cjmayo commented Apr 10, 2019

No description provided.

import pytest
from contextlib import contextmanager
from linkcheck import LinkCheckerInterrupt
from builtins import str
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm not familiar with future. Is this equivalent to str = unicode on Python 2?

I'm not sure I like that, it seems very subtle. I much prefer the approach six took where you have a new six.text_type to mean unicode on Python 2 and str on Python 3.

(Still, this is test code, I'm not going to veto anything.)

@cjmayo
Copy link
Copy Markdown
Contributor Author

cjmayo commented Apr 11, 2019

Rebased on master for build checks.

@cjmayo
Copy link
Copy Markdown
Contributor Author

cjmayo commented Apr 19, 2019

Rebased again and updated for as str_text

Copy link
Copy Markdown
Contributor

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

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

LGTM if you fix that one thing

pytest.skip("%s is not available" % name)
return func(*args, **kwargs)
newfunc.func_name = func.func_name
newfunc.__name = func.__name__
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Typo: __name should be __name__.

In fact I suggest using @functools.wraps(func) to decorate the def newfunc instead of copying metadata by hand. It'll also take care to copy __doc__ and __module__ and all sorts of other important things.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good to know. Added wraps.

Copy link
Copy Markdown
Contributor

@mgedmin mgedmin left a comment

Choose a reason for hiding this comment

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

LGTM!

@anarcat anarcat merged commit 7767bc5 into linkchecker:master Apr 25, 2019
@cjmayo cjmayo deleted the python3_06 branch November 21, 2019 19:48
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.

4 participants