[Freeipa-devel] [freeipa PR#475][opened] Add options to run only ipaclient unittests

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Author: tiran
Title: #475: Add options to run only ipaclient unittests
Action: opened

PR body:
"""
A new option for ipa-run-tests makes the test runner ignore
subdirectories or skips tests that depend on the ipaserver package or on
a running framework for RPC integration tests. The new option enables
testing of client-only builds.

$ ipatests/ipa-run-tests --ipaclient-unittests
...
platform linux2 -- Python 2.7.13, pytest-2.9.2, py-1.4.32, pluggy-0.3.1
rootdir: /home/heimes/redhat, inifile: tox.ini
plugins: sourceorder-0.5, cov-2.3.0, betamax-0.7.1, multihost-1.1
collected 451 items

test_util.py ........
util.py ..
test_ipaclient/test_csrgen.py ..............ssss...
test_ipalib/test_aci.py ...................
test_ipalib/test_backend.py ........
test_ipalib/test_base.py ...............
test_ipalib/test_capabilities.py .
test_ipalib/test_cli.py ...
test_ipalib/test_config.py ...............
test_ipalib/test_crud.py ...............
test_ipalib/test_errors.py .......
test_ipalib/test_frontend.py ........................................
test_ipalib/test_messages.py ....
test_ipalib/test_output.py ...
test_ipalib/test_parameters.py .............................................................
test_ipalib/test_plugable.py ........
test_ipalib/test_rpc.py ......ssssssss
test_ipalib/test_text.py .............................
test_ipalib/test_x509.py ...
test_ipapython/test_cookie.py ............
test_ipapython/test_dn.py ...........................
test_ipapython/test_ipautil.py ..................................................................
test_ipapython/test_ipavalidate.py ..........
test_ipapython/test_kerberos.py ..............
test_ipapython/test_keyring.py ..........
test_ipapython/test_ssh.py ...............................
test_pkcs10/test_pkcs10.py .....

https://fedorahosted.org/freeipa/ticket/6517

Signed-off-by: Christian Heimes <***@redhat.com>
"""

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/475/head:pr475
git checkout pr475

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

tiran commented:
"""
PS: I'm not attached to the new of the option. Please speak up if you can come up with a better name than ```--ipaclient-unittests```.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-280578416

[martbab]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

martbab commented:
"""
I was thinking that instead of making up more options to test runner we could reorganize the `ipatests/` directory to actually make sense from the consumer's POV, although I admit that it will take more time and also has potential to break are incredibly... fragile test handling.

On the plus side, you would run the tests you want naturally by just specifying the path that interests you and let the test discovery do the rest.

A silly example:

```bash

$ ipa-run-tests test_ipaclient/test_units
test_ipaclient/test_units/test_util.py ........
test_ipaclient/test_units/tutil.py ..
test_ipaclient/test_units/test_csrgen.py ..............ssss...
...
```
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-280579653

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

tiran commented:
"""
My first PoC used the approach. ipa-run-tests can already takes arguments to limit tests to subdirectories. One has to remember that ipa-run-tests performs chdir()...

The additional option combined with the marker have some benefits. It's not just less work and more robust, it permits us to use a cleaner and declarative way to mark test cases. The author of a test case can mark a test with pytest's standard API instead of changing some test runner options. The markers allow us to skip some test cases in a file, too. In my and your example, some tests of ```test_csrgen``` are skipped in ```--ipaclient-unittest``` mode. :)
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-280582317

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Author: tiran
Title: #475: Add options to run only ipaclient unittests
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/475/head:pr475
git checkout pr475

[martbab]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

martbab commented:
"""
I am not a big fan of mixing filename matching and markers in this PR. I feel that using only one of those approaches is a more cleaner solution and it seems that marking all the tests and then running a subset using the pytest's marker selection API loks like the easiest road.

It seems like a daunting task but it may actually be easier given that you can mark whole modules[1] or even generate marker dynamically by introspecting node IDs during test collection[2].

You can ultimately provide an option as an alias for selecting/deselecting markers as needed if you like but the underlying implementation will be cleaner as result.

[1] http://doc.pytest.org/en/latest/example/markers.html#marking-whole-classes-or-modules
[2] http://doc.pytest.org/en/latest/example/markers.html#automatically-adding-markers-based-on-test-names
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-283073142

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

tiran commented:
"""
I'm not a big fan either. Can you come up with a better solution that does not result in import errors? Because the module marker or class markers still import the whole module. For client-only tests, ipaserver is not available. For Python packaging builds, neither ipaserver nor ipaplatform are available.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-283079924

[martbab]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

martbab commented:
"""
Oh my, every time I think about something nice that should work there is some corner case that ruins it.

I guess that one way to work around it would be to keep the `try: ... except importError` guards in the offending modules and add skip markers like `@pytest.mark.skipif(ipaserver is None, "ipaserver module unavailable")` or skip whole modules.

As a side note, I really wish that our test suite would be a little less... um, special.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-283084101

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Author: tiran
Title: #475: Add options to run only ipaclient unittests
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/475/head:pr475
git checkout pr475

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

tiran commented:
"""
I pushed an alternative approach that checks for the option and raises skip in packages. It needs some extra workaround in the integration plugin.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-283098644

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Author: tiran
Title: #475: Add options to run only ipaclient unittests
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/475/head:pr475
git checkout pr475

[martbab]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

martbab commented:
"""
I like the second approach better. If you squash the commits I will Ack the PR. I still think we need a substantial reorganization of the test suites but that needs more consideration and time.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-283978683

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Author: tiran
Title: #475: Add options to run only ipaclient unittests
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/475/head:pr475
git checkout pr475

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

tiran commented:
"""
@martbab I've cleanup up some white space noise and squashed all commits.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-286595358

[tiran]

URL: https://github.com/freeipa/freeipa/pull/475
Author: tiran
Title: #475: Add options to run only ipaclient unittests
Action: synchronized

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/475/head:pr475
git checkout pr475

[martbab]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

martbab commented:
"""
I have one small question and am going to try out some integration tests to see if we did not break something in them as Travis won't catch that.
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-287287386

[martbab]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

Label: +ack

[martbab]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

Label: +pushed

[martbab]

URL: https://github.com/freeipa/freeipa/pull/475
Title: #475: Add options to run only ipaclient unittests

martbab commented:
"""
master:

* fd1b4f6ec9a349196d5df510008c4745f0b1fb84 Add options to run only ipaclient unittests
ipa-4-5:

* 29b885a8fac82e963f5ab98d178e81854056930e Add options to run only ipaclient unittests
"""

See the full comment at https://github.com/freeipa/freeipa/pull/475#issuecomment-287362273

[martbab]

URL: https://github.com/freeipa/freeipa/pull/475
Author: tiran
Title: #475: Add options to run only ipaclient unittests
Action: closed

To pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/475/head:pr475
git checkout pr475