Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
On 09.12.2015 12:40, Martin Babinsky wrote: On 12/09/2015 11:29 AM, Lenka Doudova wrote: On 12/09/2015 10:13 AM, Martin Basti wrote: On 09.12.2015 09:41, Lenka Doudova wrote: Hi, attaching fixed patches for master and ipa-4-2 branch. Also fixes test accordingly to https://fedorahosted.org/freeipa/ticket/5387. Lenka On 11/20/2015 12:13 PM, Martin Babinsky wrote: On 11/19/2015 10:34 AM, Petr Viktorin wrote: On 11/19/2015 09:30 AM, Lenka Doudova wrote: On 11/18/2015 04:51 PM, Martin Babinsky wrote: On 11/18/2015 02:16 PM, Lenka Doudova wrote: Hi, here's a patch that adds a few comments to stageuser tests in order to allow easier determining of a problem when tests fail. Lenka Hi Lenka, Firstly a technical detail: Python indexes lists from 0, so the comments in 'options_ok' do not correctly map to the test names anyway. I am also not sure if this patch is worth reviewing and pushing as it IMHO doesn't help in the identification of failed tests at all. This should be solved at more fundamental level. Ouch, good point, I didn't realize. Sorry. Anyway, the issue is that even if tests are run in verbose mode, you get output like this: ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] PASSED If some test fails, you can't really tell which command was the one responsible for the fail. This should be solved by https://fedorahosted.org/freeipa/ticket/5449. Until that's done, though, the only way to find out which command failed is looking at the code and finding which parameters were put into the command, which was not much possible without better commenting the test case (as I realized last week when David Kupka asked me to help him find the reason for failed tests). Obviously I can rewrite the tests so there's 27 separate test cases, one for each parameter, instead of one method that 'unwraps' into 27 test cases, which would entirely eliminate the confusion. So far I don't know of a way to put 27 similar test cases in one method which would allow easy recognition of the test cases. I'll wait with fixing the patch until further discussion. Hello, Pytest wants you to be a bit more explicit about how to name the parameters. (It "hides" dicts by default, because large dicts would make the output even more confusing than the numbers.) Please try the attached patch. Docs are at https://pytest.org/latest/fixture.html#parametrizing-a-fixture This looks like a better approach IMHO, you can then see which attribute/value was being checked. I would very much favor more descriptive test/fixture names in the beginning. Hello, we use usually bottom posting on freeipa-devel please try to keep reply in this way. Patch: I do not like the idea of separated lists, IMO it is hard to manage and is easy to create mistakes. How about this (untested, just from top of my head): http://fpaste.org/298994/65184014/ Martin Great idea, thanks. Fixed patches attached. Lenka Tests pass and code looks good, ACK. Pushed to master: a66a2c5160dbc23cdeec55d17422812028939e16 Pushed to ipa-4-2: 75675fc71a148dcc17b025a80123aa01644f5297 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
On 12/09/2015 11:29 AM, Lenka Doudova wrote: On 12/09/2015 10:13 AM, Martin Basti wrote: On 09.12.2015 09:41, Lenka Doudova wrote: Hi, attaching fixed patches for master and ipa-4-2 branch. Also fixes test accordingly to https://fedorahosted.org/freeipa/ticket/5387. Lenka On 11/20/2015 12:13 PM, Martin Babinsky wrote: On 11/19/2015 10:34 AM, Petr Viktorin wrote: On 11/19/2015 09:30 AM, Lenka Doudova wrote: On 11/18/2015 04:51 PM, Martin Babinsky wrote: On 11/18/2015 02:16 PM, Lenka Doudova wrote: Hi, here's a patch that adds a few comments to stageuser tests in order to allow easier determining of a problem when tests fail. Lenka Hi Lenka, Firstly a technical detail: Python indexes lists from 0, so the comments in 'options_ok' do not correctly map to the test names anyway. I am also not sure if this patch is worth reviewing and pushing as it IMHO doesn't help in the identification of failed tests at all. This should be solved at more fundamental level. Ouch, good point, I didn't realize. Sorry. Anyway, the issue is that even if tests are run in verbose mode, you get output like this: ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] PASSED If some test fails, you can't really tell which command was the one responsible for the fail. This should be solved by https://fedorahosted.org/freeipa/ticket/5449. Until that's done, though, the only way to find out which command failed is looking at the code and finding which parameters were put into the command, which was not much possible without better commenting the test case (as I realized last week when David Kupka asked me to help him find the reason for failed tests). Obviously I can rewrite the tests so there's 27 separate test cases, one for each parameter, instead of one method that 'unwraps' into 27 test cases, which would entirely eliminate the confusion. So far I don't know of a way to put 27 similar test cases in one method which would allow easy recognition of the test cases. I'll wait with fixing the patch until further discussion. Hello, Pytest wants you to be a bit more explicit about how to name the parameters. (It "hides" dicts by default, because large dicts would make the output even more confusing than the numbers.) Please try the attached patch. Docs are at https://pytest.org/latest/fixture.html#parametrizing-a-fixture This looks like a better approach IMHO, you can then see which attribute/value was being checked. I would very much favor more descriptive test/fixture names in the beginning. Hello, we use usually bottom posting on freeipa-devel please try to keep reply in this way. Patch: I do not like the idea of separated lists, IMO it is hard to manage and is easy to create mistakes. How about this (untested, just from top of my head): http://fpaste.org/298994/65184014/ Martin Great idea, thanks. Fixed patches attached. Lenka Tests pass and code looks good, ACK. -- Martin^3 Babinsky -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
On 12/09/2015 10:13 AM, Martin Basti wrote:
On 09.12.2015 09:41, Lenka Doudova wrote:
Hi,
attaching fixed patches for master and ipa-4-2 branch.
Also fixes test accordingly to
https://fedorahosted.org/freeipa/ticket/5387.
Lenka
On 11/20/2015 12:13 PM, Martin Babinsky wrote:
On 11/19/2015 10:34 AM, Petr Viktorin wrote:
On 11/19/2015 09:30 AM, Lenka Doudova wrote:
On 11/18/2015 04:51 PM, Martin Babinsky wrote:
On 11/18/2015 02:16 PM, Lenka Doudova wrote:
Hi,
here's a patch that adds a few comments to stageuser tests in
order to
allow easier determining of a problem when tests fail.
Lenka
Hi Lenka,
Firstly a technical detail: Python indexes lists from 0, so the
comments in 'options_ok' do not correctly map to the test names
anyway.
I am also not sure if this patch is worth reviewing and pushing
as it
IMHO doesn't help in the identification of failed tests at all.
This should be solved at more fundamental level.
Ouch, good point, I didn't realize. Sorry.
Anyway, the issue is that even if tests are run in verbose mode,
you get
output like this:
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27]
PASSED
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28]
PASSED
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29]
PASSED
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210]
PASSED
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211]
PASSED
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212]
PASSED
If some test fails, you can't really tell which command was the one
responsible for the fail. This should be solved by
https://fedorahosted.org/freeipa/ticket/5449. Until that's done,
though,
the only way to find out which command failed is looking at the
code and
finding which parameters were put into the command, which was not
much
possible without better commenting the test case (as I realized last
week when David Kupka asked me to help him find the reason for failed
tests).
Obviously I can rewrite the tests so there's 27 separate test
cases, one
for each parameter, instead of one method that 'unwraps' into 27 test
cases, which would entirely eliminate the confusion. So far I
don't know
of a way to put 27 similar test cases in one method which would allow
easy recognition of the test cases.
I'll wait with fixing the patch until further discussion.
Hello,
Pytest wants you to be a bit more explicit about how to name the
parameters. (It "hides" dicts by default, because large dicts would
make
the output even more confusing than the numbers.)
Please try the attached patch.
Docs are at
https://pytest.org/latest/fixture.html#parametrizing-a-fixture
This looks like a better approach IMHO, you can then see which
attribute/value was being checked.
I would very much favor more descriptive test/fixture names in the
beginning.
Hello,
we use usually bottom posting on freeipa-devel please try to keep
reply in this way.
Patch:
I do not like the idea of separated lists, IMO it is hard to manage
and is easy to create mistakes.
How about this (untested, just from top of my head):
http://fpaste.org/298994/65184014/
Martin
Great idea, thanks. Fixed patches attached.
Lenka
From 646db80b7cc0912c310615d804fcb1561e19961f Mon Sep 17 00:00:00 2001
From: Lenka Doudova
Date: Mon, 23 Nov 2015 10:27:07 +0100
Subject: [PATCH] Adding descriptive IDs to stageuser tests
Adding descriptive IDs to parametrized stageuser test for better identification of test cases.
---
ipatests/test_xmlrpc/test_stageuser_plugin.py| 84 ++--
ipatests/test_xmlrpc/tracker/stageuser_plugin.py | 2 +-
ipatests/test_xmlrpc/tracker/user_plugin.py | 9 ++-
3 files changed, 56 insertions(+), 39 deletions(-)
diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index 4eb968451f926ca0ee8fa5aeae1a96770f56eb45..42ecf046884369bd74b03194937c425215b99f6c 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -15,6 +15,7 @@ import pytest
import six
+from collections import OrderedDict
from ipalib import api, errors
from ipatests.test_xmlrpc import objectclasses
@@ -53,35 +54,40 @@ sshpubkey = (u'ssh-rsa B3NzaC1yc2EDAQABAAABAQDGAX3xAeLeaJggwTqMjxNwa6X'
sshpubkeyfp = (u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B '
'public key test (ssh-rsa)')
-options_ok = [
-{u'cn': u'name'},
-{u'initials': u'in'},
-{u'displayname': u'display'},
-{u'homedirectory': u'/home/homedir'},
-{u'gecos': u'gecos'},
-{u'loginshell': u'/bin/shell'},
-{u'mail': u'email@email.email'},
-{u'title': u'newbie'},
-{u'krbprincipalname': u'kerberos@%s'
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
On 09.12.2015 09:41, Lenka Doudova wrote: Hi, attaching fixed patches for master and ipa-4-2 branch. Also fixes test accordingly to https://fedorahosted.org/freeipa/ticket/5387. Lenka On 11/20/2015 12:13 PM, Martin Babinsky wrote: On 11/19/2015 10:34 AM, Petr Viktorin wrote: On 11/19/2015 09:30 AM, Lenka Doudova wrote: On 11/18/2015 04:51 PM, Martin Babinsky wrote: On 11/18/2015 02:16 PM, Lenka Doudova wrote: Hi, here's a patch that adds a few comments to stageuser tests in order to allow easier determining of a problem when tests fail. Lenka Hi Lenka, Firstly a technical detail: Python indexes lists from 0, so the comments in 'options_ok' do not correctly map to the test names anyway. I am also not sure if this patch is worth reviewing and pushing as it IMHO doesn't help in the identification of failed tests at all. This should be solved at more fundamental level. Ouch, good point, I didn't realize. Sorry. Anyway, the issue is that even if tests are run in verbose mode, you get output like this: ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] PASSED If some test fails, you can't really tell which command was the one responsible for the fail. This should be solved by https://fedorahosted.org/freeipa/ticket/5449. Until that's done, though, the only way to find out which command failed is looking at the code and finding which parameters were put into the command, which was not much possible without better commenting the test case (as I realized last week when David Kupka asked me to help him find the reason for failed tests). Obviously I can rewrite the tests so there's 27 separate test cases, one for each parameter, instead of one method that 'unwraps' into 27 test cases, which would entirely eliminate the confusion. So far I don't know of a way to put 27 similar test cases in one method which would allow easy recognition of the test cases. I'll wait with fixing the patch until further discussion. Hello, Pytest wants you to be a bit more explicit about how to name the parameters. (It "hides" dicts by default, because large dicts would make the output even more confusing than the numbers.) Please try the attached patch. Docs are at https://pytest.org/latest/fixture.html#parametrizing-a-fixture This looks like a better approach IMHO, you can then see which attribute/value was being checked. I would very much favor more descriptive test/fixture names in the beginning. Hello, we use usually bottom posting on freeipa-devel please try to keep reply in this way. Patch: I do not like the idea of separated lists, IMO it is hard to manage and is easy to create mistakes. How about this (untested, just from top of my head): http://fpaste.org/298994/65184014/ Martin -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
Hi,
attaching fixed patches for master and ipa-4-2 branch.
Also fixes test accordingly to https://fedorahosted.org/freeipa/ticket/5387.
Lenka
On 11/20/2015 12:13 PM, Martin Babinsky wrote:
On 11/19/2015 10:34 AM, Petr Viktorin wrote:
On 11/19/2015 09:30 AM, Lenka Doudova wrote:
On 11/18/2015 04:51 PM, Martin Babinsky wrote:
On 11/18/2015 02:16 PM, Lenka Doudova wrote:
Hi,
here's a patch that adds a few comments to stageuser tests in
order to
allow easier determining of a problem when tests fail.
Lenka
Hi Lenka,
Firstly a technical detail: Python indexes lists from 0, so the
comments in 'options_ok' do not correctly map to the test names
anyway.
I am also not sure if this patch is worth reviewing and pushing as it
IMHO doesn't help in the identification of failed tests at all.
This should be solved at more fundamental level.
Ouch, good point, I didn't realize. Sorry.
Anyway, the issue is that even if tests are run in verbose mode, you
get
output like this:
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27]
PASSED
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28]
PASSED
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29]
PASSED
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210]
PASSED
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211]
PASSED
ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212]
PASSED
If some test fails, you can't really tell which command was the one
responsible for the fail. This should be solved by
https://fedorahosted.org/freeipa/ticket/5449. Until that's done,
though,
the only way to find out which command failed is looking at the code
and
finding which parameters were put into the command, which was not much
possible without better commenting the test case (as I realized last
week when David Kupka asked me to help him find the reason for failed
tests).
Obviously I can rewrite the tests so there's 27 separate test cases,
one
for each parameter, instead of one method that 'unwraps' into 27 test
cases, which would entirely eliminate the confusion. So far I don't
know
of a way to put 27 similar test cases in one method which would allow
easy recognition of the test cases.
I'll wait with fixing the patch until further discussion.
Hello,
Pytest wants you to be a bit more explicit about how to name the
parameters. (It "hides" dicts by default, because large dicts would make
the output even more confusing than the numbers.)
Please try the attached patch.
Docs are at
https://pytest.org/latest/fixture.html#parametrizing-a-fixture
This looks like a better approach IMHO, you can then see which
attribute/value was being checked.
I would very much favor more descriptive test/fixture names in the
beginning.
From 163bbd04cf02d278b3d4ef8d0ee91878270e4377 Mon Sep 17 00:00:00 2001
From: Lenka Doudova
Date: Mon, 23 Nov 2015 10:27:07 +0100
Subject: [PATCH] Adding descriptive IDs to stageuser tests
Adding descriptive IDs to parametrized stageuser test for better identification of test cases.
---
ipatests/test_xmlrpc/test_stageuser_plugin.py| 30 ++--
ipatests/test_xmlrpc/tracker/stageuser_plugin.py | 2 +-
ipatests/test_xmlrpc/tracker/user_plugin.py | 9 +--
3 files changed, 31 insertions(+), 10 deletions(-)
diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index 4eb968451f926ca0ee8fa5aeae1a96770f56eb45..bda738b786f99296deaeb192e84965bb3e97cfea 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -83,6 +83,16 @@ options_ok = [
{u'random': True},
]
+options_ids = [
+'full name', 'initials', 'display name', 'home directory', 'GECOS',
+'shell', 'email address', 'job title', 'kerberos principal',
+'uppercase kerberos principal', 'street address', 'city', 'state',
+'zip code', 'telephone number', 'fax number', 'mobile tel. number',
+'pager number', 'organizational unit', 'car license', 'SSH key',
+'manager', 'user ID number', 'group ID number', 'UID and GID numbers',
+'password', 'random password'
+]
+
@pytest.fixture(scope='class')
def stageduser(request):
@@ -90,13 +100,19 @@ def stageduser(request):
return tracker.make_fixture(request)
-@pytest.fixture(scope='class', params=options_ok)
+@pytest.fixture(scope='class', params=options_ok, ids=options_ids)
def stageduser2(request):
tracker = StageUserTracker(u'suser2', u'staged', u'user', **request.param)
return tracker.make_fixture_activate(request)
@pytest.fixture(scope='class')
+def user_activated(request):
+tracker = UserTracker(u'suser2', u'staged', u'user')
+return tracker.make_fixture(request)
+
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
On 11/19/2015 10:34 AM, Petr Viktorin wrote: On 11/19/2015 09:30 AM, Lenka Doudova wrote: On 11/18/2015 04:51 PM, Martin Babinsky wrote: On 11/18/2015 02:16 PM, Lenka Doudova wrote: Hi, here's a patch that adds a few comments to stageuser tests in order to allow easier determining of a problem when tests fail. Lenka Hi Lenka, Firstly a technical detail: Python indexes lists from 0, so the comments in 'options_ok' do not correctly map to the test names anyway. I am also not sure if this patch is worth reviewing and pushing as it IMHO doesn't help in the identification of failed tests at all. This should be solved at more fundamental level. Ouch, good point, I didn't realize. Sorry. Anyway, the issue is that even if tests are run in verbose mode, you get output like this: ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] PASSED If some test fails, you can't really tell which command was the one responsible for the fail. This should be solved by https://fedorahosted.org/freeipa/ticket/5449. Until that's done, though, the only way to find out which command failed is looking at the code and finding which parameters were put into the command, which was not much possible without better commenting the test case (as I realized last week when David Kupka asked me to help him find the reason for failed tests). Obviously I can rewrite the tests so there's 27 separate test cases, one for each parameter, instead of one method that 'unwraps' into 27 test cases, which would entirely eliminate the confusion. So far I don't know of a way to put 27 similar test cases in one method which would allow easy recognition of the test cases. I'll wait with fixing the patch until further discussion. Hello, Pytest wants you to be a bit more explicit about how to name the parameters. (It "hides" dicts by default, because large dicts would make the output even more confusing than the numbers.) Please try the attached patch. Docs are at https://pytest.org/latest/fixture.html#parametrizing-a-fixture This looks like a better approach IMHO, you can then see which attribute/value was being checked. I would very much favor more descriptive test/fixture names in the beginning. -- Martin^3 Babinsky -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
On 11/19/2015 09:30 AM, Lenka Doudova wrote: > On 11/18/2015 04:51 PM, Martin Babinsky wrote: >> On 11/18/2015 02:16 PM, Lenka Doudova wrote: >>> Hi, >>> >>> here's a patch that adds a few comments to stageuser tests in order to >>> allow easier determining of a problem when tests fail. >>> >>> Lenka >>> >>> >> >> Hi Lenka, >> >> Firstly a technical detail: Python indexes lists from 0, so the >> comments in 'options_ok' do not correctly map to the test names anyway. >> >> I am also not sure if this patch is worth reviewing and pushing as it >> IMHO doesn't help in the identification of failed tests at all. >> >> This should be solved at more fundamental level. >> > Ouch, good point, I didn't realize. Sorry. > > Anyway, the issue is that even if tests are run in verbose mode, you get > output like this: > > ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] > PASSED > > ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] > PASSED > > ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] > PASSED > > ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] > PASSED > > ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] > PASSED > > ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] > PASSED > > > If some test fails, you can't really tell which command was the one > responsible for the fail. This should be solved by > https://fedorahosted.org/freeipa/ticket/5449. Until that's done, though, > the only way to find out which command failed is looking at the code and > finding which parameters were put into the command, which was not much > possible without better commenting the test case (as I realized last > week when David Kupka asked me to help him find the reason for failed > tests). > Obviously I can rewrite the tests so there's 27 separate test cases, one > for each parameter, instead of one method that 'unwraps' into 27 test > cases, which would entirely eliminate the confusion. So far I don't know > of a way to put 27 similar test cases in one method which would allow > easy recognition of the test cases. > I'll wait with fixing the patch until further discussion. Hello, Pytest wants you to be a bit more explicit about how to name the parameters. (It "hides" dicts by default, because large dicts would make the output even more confusing than the numbers.) Please try the attached patch. Docs are at https://pytest.org/latest/fixture.html#parametrizing-a-fixture -- Petr Viktorin From 1d36f052102942dcf3dab24bf5ede504ec94ca2f Mon Sep 17 00:00:00 2001 From: Petr Viktorin Date: Thu, 19 Nov 2015 10:27:47 +0100 Subject: [PATCH] stageuser tests: Use "str" to display fixture parameter names --- ipatests/test_xmlrpc/test_stageuser_plugin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py index 43c59b7c7ec2902064fc363c66e98cc98e8b9e17..21bfe5f7fa9a39e3af1cdb14c48e821901765d39 100644 --- a/ipatests/test_xmlrpc/test_stageuser_plugin.py +++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py @@ -329,7 +329,7 @@ def stageduser(request): return tracker.make_fixture(request) -@pytest.fixture(scope='class', params=options_ok) +@pytest.fixture(scope='class', params=options_ok, ids=str) def stageduser2(request): tracker = StageUserTracker(u'suser2', u'staged', u'user', **request.param) return tracker.make_fixture_activate(request) -- 2.1.0 -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
On 11/18/2015 04:51 PM, Martin Babinsky wrote: On 11/18/2015 02:16 PM, Lenka Doudova wrote: Hi, here's a patch that adds a few comments to stageuser tests in order to allow easier determining of a problem when tests fail. Lenka Hi Lenka, Firstly a technical detail: Python indexes lists from 0, so the comments in 'options_ok' do not correctly map to the test names anyway. I am also not sure if this patch is worth reviewing and pushing as it IMHO doesn't help in the identification of failed tests at all. This should be solved at more fundamental level. Ouch, good point, I didn't realize. Sorry. Anyway, the issue is that even if tests are run in verbose mode, you get output like this: ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] PASSED ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] PASSED If some test fails, you can't really tell which command was the one responsible for the fail. This should be solved by https://fedorahosted.org/freeipa/ticket/5449. Until that's done, though, the only way to find out which command failed is looking at the code and finding which parameters were put into the command, which was not much possible without better commenting the test case (as I realized last week when David Kupka asked me to help him find the reason for failed tests). Obviously I can rewrite the tests so there's 27 separate test cases, one for each parameter, instead of one method that 'unwraps' into 27 test cases, which would entirely eliminate the confusion. So far I don't know of a way to put 27 similar test cases in one method which would allow easy recognition of the test cases. I'll wait with fixing the patch until further discussion. Lenka -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests
On 11/18/2015 02:16 PM, Lenka Doudova wrote: Hi, here's a patch that adds a few comments to stageuser tests in order to allow easier determining of a problem when tests fail. Lenka Hi Lenka, Firstly a technical detail: Python indexes lists from 0, so the comments in 'options_ok' do not correctly map to the test names anyway. I am also not sure if this patch is worth reviewing and pushing as it IMHO doesn't help in the identification of failed tests at all. This should be solved at more fundamental level. -- Martin^3 Babinsky -- Manage your subscription for the Freeipa-devel mailing list: https://www.redhat.com/mailman/listinfo/freeipa-devel Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code
