Issue 3082 - Separate test code from lib389

[stanislavlevin]

Bug Description:
topologies module is used only in tests. But because it's part of
lib389, it pulls in pytest and its dependencies. Additionally, lib389
contains tests that are installed, but not needed for lib389 to
function.

perftools module is also used only for tests and depends on
topologies.

Fix Description:
topologies and perftools modules have been converted into Python
packages.

Fixes: #3082

Reviewed by:

[progier389]

Avoiding pytest dependency from lib389 is a good idea but I do not like the idea of delivering only parts of src/lib389/lib389/*
IMHO to be clean and easier to maintain the code, the pytest fixture definitions should be moved away from src/lib389/lib389/* and back to somewhere in dirserv/tests/*.
Then perftools and topologies.py could stay in the lib389
BTW: we may need perftoolspy if we ever decide to deliver the cli/dsrate utility...

[droideck]

Avoiding pytest dependency from lib389 is a good idea but I do not like the idea of delivering only parts of src/lib389/lib389/* IMHO to be clean and easier to maintain the code, the pytest fixture definitions should be moved away from src/lib389/lib389/* and back to somewhere in dirserv/tests/*. Then perftools and topologies.py could stay in the lib389 BTW: we may need perftoolspy if we ever decide to deliver the cli/dsrate utility...

+1

Also, the PR can affect our QE and CI significantly. @sgouvern could you please check if it's what you want? I recall we had some plans regards the topologies/tests code moving...

[mreynolds389]

Also, the PR can affect our QE and CI significantly. @sgouvern could you please check if it's what you want? I recall we had some plans regards the topologies/tests code moving...

We already have a ticket for it:

#3082

Looks like @stanislavlevin already did PR for it that strips out the tests, but I think we want to "move" the tests outside the lib389 source. We are not running them from where they are now anyway.

[stanislavlevin]

The goal of this PR is getting rid of Pytest as dependency of lib389 and pointless ( in terms of distro packaging ) lib389.tests. I agree that the best approach is splitting lib389.topologies into tests-only and production code.
But for example

[someuser somedir]$ git grep 'from lib389.topologies' dirsrvtests/ | wc -l
343

That's why I chose the lesser evil. If everyone is Ok with fixing dirsrvtests I can do it.

[sgouvern]

We'll need a little more time to look more deeply to what it means for QE and CIs

[stanislavlevin]

ofc, no reason to rush, please let me know when and if you want my help here, thank you.

[bsimonova]

@sgouvern @vashirov
I tested the change and if I understand correctly, for now we should just make sure to run the tests with "PYTHONPATH=src/lib389" and cover it in our github actions (gating already covered).
Or is it more complicated in actions?

[vashirov]

This is a somewhat complicated topic.

@progier389

Avoiding pytest dependency from lib389 is a good idea but I do not like the idea of delivering only parts of src/lib389/lib389/* IMHO to be clean and easier to maintain the code, the pytest fixture definitions should be moved away from src/lib389/lib389/* and back to somewhere in dirserv/tests/*.

lib389 has 2 types of tests: unit and integration. Integration tests use topologies module and should be moved to dirsrvtests. As it is right now, there are a lot of broken tests, they don't run under tox and under non-root account. There is a ticket to fix them: #5005
Unit tests must be kept inside lib389 tree, but should not be shipped in downstream. So I agree with the change that strips them and drops pytest dependency.

Then perftools and topologies.py could stay in the lib389

The topologies module doesn't have any production code, it really should be moved to dirsrvtests.
perftools module uses topologies module. It creates its own instance, populates it with data, so it's not a production code either (i.e. it should not be executed against an existing production instance as it does some destructive actions), and I'd rather move it to dirsrvtests too.

BTW: we may need perftoolspy if we ever decide to deliver the cli/dsrate utility...

In this case I'd separate perftools code into test and production code. IMHO dsrate should run on a provided instance rather than create one. But that's a completely different discussion.

@bsimonova
I'd rather avoid PYTHONPATH trick, as it has side effects. We already do some gymnastics in the CLU test suite, where we undefine externally provided PYTHONPATH so that ds* tools use system lib389 instead. In downstream we used PYTHONPATH because we used tests from master branch, but since it's no longer the case, I'd prefer to drop it there as well. So that a system lib389 would be always used.

[bsimonova]

@vashirov @sgouvern
Thanks for the explanation.

So if the change stays as is then we need to change the imports to:

from src.lib389.lib389.topologies import topology_st

for the tests to work. Unless we want move topologies to dirsrvtests directory right in this PR.
If I understand it correctly.

[vashirov]

With this change topologies.py won't be present in the rpm:

rpm -qpl python3-lib389-2.1.0-202204041450git43aa0238b.fc35.noarch.rpm | grep topologies -c
0

So that's why we either need to use PYTHONPATH, or move topologies module to dirsrvtests (and modify all tests).