Repository navigation
Conversation
| from typing import TYPE_CHECKING, Any, Callable, TypeVar, cast | ||
| from urllib.parse import quote_plus | ||
|
|
||
| import pytest |
There was a problem hiding this comment.
This is the only change I am unsure about:
slapdtest is packaged, e.g. in PyPI or Debian python3-ldap in the main distribution.
But adding a dependency on pytest in the core-library is not correct.
I added a extra-dependency for the project:
[project.optional-dependencies]
slapdtest = ["pytest"]Debian could e.g. add some binary package python3-ldap+slapdtest (that's how they manager rust-extras - don't know if that's a valid Python-way).
Or they can split the binary package and make a python3-slapdtest - should be doable with less dh-python effort.
I don't know about other distributions.
So, I could also add a deferred import or try-except-ImportError construct.
Opinions?
There was a problem hiding this comment.
Sure, for Fedora, I'd keep slapdtest in python3-ldap and add a python3-ldap+slapdtest extras metapackage to pull in pytest, following something like Hypothesis's packaging did.
I'd also move import pytest into the skip branch of skip_unless_ci(), so consumers using just SlapdObject don't need pytest. NumPy uses a similar deferred import in its test runner.
a05d3e2 to
9f2c55b
Compare
a467039 to
504cacb
Compare
| @@ -1,5 +1,6 @@ | |||
| import os | |||
There was a problem hiding this comment.
Would you be okay with renaming all files to test_-prefix?
There was a problem hiding this comment.
Yeah, test_ makes sense and that's how pytest also discovers things. So I would remove the custom python_files = t_*.py setting and would update the explicit file paths in tox.ini along with the rename.
@mistotebe WDYT?
There was a problem hiding this comment.
OK, I added an additional commit for the rename.
There was a problem hiding this comment.
I think you might forgot to push it :)
There was a problem hiding this comment.
Correct, I only pushed it into my local fork. Now it's there.
504cacb to
30ad414
Compare
30ad414 to
26f2277
Compare
Issue: #396