Skip to content

test: migrate to pytest - #669

Open
spaceone wants to merge 4 commits into
mainfrom
pytest-migration
Open

spaceone wants to merge 4 commits into
mainfrom
pytest-migration

Conversation

@spaceone

@spaceone spaceone commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

Issue: #396

Comment thread Lib/slapdtest/_slapdtest.py Outdated
from typing import TYPE_CHECKING, Any, Callable, TypeVar, cast
from urllib.parse import quote_plus

import pytest

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.

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?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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.

@spaceone
spaceone force-pushed the feat/asynchronous-sasl-binds branch from a05d3e2 to 9f2c55b Compare October 9, 2026 19:28
@spaceone
spaceone requested review from droideck and mistotebe and removed request for droideck October 9, 2026 19:51
Comment thread Tests/test_bind.py
@@ -1,5 +1,6 @@
import os

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.

Would you be okay with renaming all files to test_-prefix?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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?

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.

OK, I added an additional commit for the rename.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think you might forgot to push it :)

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.

Correct, I only pushed it into my local fork. Now it's there.

to be compliant with pytest defaults

Issue: #396
Add pytest as an optional dependency/extra for consumers that use the
`slapdtest` module shipped with the package. The ldap package
does not require pytest at runtime.
for compatibility with standard pytest.

Issue: #396
@spaceone
spaceone changed the base branch from feat/asynchronous-sasl-binds to main October 21, 2026 08:26
@spaceone spaceone added this to the 3.5.0 milestone Oct 25, 2026
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.

2 participants