Repository navigation
Strange test name, used idstyle=None #154
Description
Python 3.6.8
pytest-6.1.2
cases-2.7.1
After solving problem #151, some tests took the name from @case(id), but others began to look strange.
Example:
test_file
@pytest.fixture
def validationOff():
app.config["validation"] = False
yield
app.config["validation"] = True
@parametrize_with_cases("inputs,request_data", prefix="case_create_", idstyle=None)
@pytest.mark.usefixtures("clearMongo", "mocked_oid", "mocked_uuid")
def test_create(db, flaskClient, inputs, request_data):
...
db, flaskClient, clearMongo, mocked_oid, mocked_uuid - fixtures
case file
class CreateCases(object):
@case(id="current_object")
def case_create_1(self):
inputs = ["foo", "bar"]
request_data = {"foo": "dfg", "data": "asd"}
return inputs, request_data
@case(id="not_valid")
@pytest.mark.usefixtures("validationOff")
def case_create_2(self):
inputs = ["foo", "bar"]
request_data = {"foo": "dfg", "data": "asd"}
return inputs, request_data
@case(id="history_object")
def case_create_3(self):
inputs = ["foo", "bar"]
request_data = {"foo": "dfg", "data": "asd"}
return inputs, request_data
@case(id="xml_without_need_namespace")
def case_create_4(self):
inputs = ["foo", "bar"]
request_data = {"foo": "dfg", "data": "asd"}
return inputs, request_data
@case(id="xml_without_need_namespace_not_valid")
@pytest.mark.usefixtures("validationOff")
def case_create_6(self):
inputs = ["foo", "bar"]
request_data = {"foo": "dfg", "data": "asd"}
return inputs, request_data
@case(id="xml_without_need_name")
def case_create_7(self):
inputs = ["foo", "bar"]
request_data = {"foo": "dfg", "data": "asd"}
return inputs, request_data
@case(id="xml_without_need_name_not_valid")
@pytest.mark.usefixtures("validationOff")
def case_create_8(self):
inputs = ["foo", "bar"]
request_data = {"foo": "dfg", "data": "asd"}
return inputs, request_data
@case(id="xml_invalid")
def case_create_9(self):
inputs = ["foo", "bar"]
request_data = {"foo": "dfg", "data": "asd"}
return inputs, request_data
Names:
test_create[current_object]
test_create[not_valid]
test_create[test_create_inputs_request_data_is_P2toP3-history_object] ?? fixture validationOff not used
test_create[test_create_inputs_request_data_is_P2toP3-xml_without_need_namespace] ?? fixture validationOff not used
test_create[xml_without_need_namespace_not_valid]
test_create[xml_without_need_name]
test_create[xml_without_need_name_not_valid]
test_create[xml_invalid]
Without idstyle=None names:
test_create[current_object]
test_create[inputs_request_data_is_not_valid]
test_create[inputs_request_data_is_P2toP3-history_object] ?? fixture validationOff not used
test_create[inputs_request_data_is_P2toP3-xml_without_need_namespace] ?? fixture validationOff not used
test_create[inputs_request_data_is_xml_without_need_namespace_not_valid]
test_create[xml_without_need_name]
test_create[inputs_request_data_is_xml_without_need_name_not_valid]
test_create[xml_invalid]
Activity
smarie commentedon Dec 2, 2020
the P2toP3 means "from parameter 2 to parameter 3". However I'm not sure it is supposed to appear when idstyle is None.
Also when idstyle is not none, it is a bit strange that "inputs_request_data_is_" is not present in all ids ?
I'll try to reproduce this one of these days and check what happens.
Do you confirm that the usefixtures mark behaves correctly here ?
arut-grigoryan commentedon Dec 2, 2020
Usefixtures mark works correctly.
As I understand it "inputs_request_data_is_" does not appear because there I am not using fixture validationOff in these tests.
smarie commentedon Dec 3, 2020
Here is a summary of the mess. I modded your example to reference a parametrized fixture too.
To convert it into a list of facts for current
@parametrizeimplem, to decide what to do:idstyleis only used when at least one parameter is a fixture reference. So on the left column we see that top and bottom lead to the same idswhen at least a fixture reference is present, a fixture union is created. So a new parameter appears upfront, doing the switch between alternatives P0, not_valid, P2-P3, xml_without_namespace_notvalid, etc. Two things make it hard/unintuitive to understand for users:
the "no style" ids
a. have a trailing underscore for fixture refs. This is due to the fact that the fixtures created automatically for the cases have an auto-generated name that should not conflict with existing symbols. For this trailing
_are created. This happens for the no style test, because the no style test appears second in the tests order. If the "explicit style" was second, it would have been the one with trailing underscores everywhereb. are not taken into account at all for parameter groups (P2toP3).
smarie commentedon Dec 3, 2020
Proposed solutions so far:
idparameter tofixture_refso that we can fix issue 3 aPending questions:
Added test for issue #154 with all styles
Added test corresponding to issue #154 but lower level (@parametrize)
Major steps towards #154 :
smarie commentedon Dec 3, 2020
I have done most of the harmonization work now with #155
The last thing that needs to be settled is the templates for the id styles. Indeed I feel that they could be improved to be more explicit. Here is a proposal using slash
/to denote a fixture union alternative'explicit' would lead to
<arg_names>/<argvalue_idx>-<argvalue_id>, so something likeinputs_request_data/P0-current_objectorinputs_request_data/P2toP3-history_object'compact' would lead to
/<argvalue_idx>-<argvalue_id>so/P0-current_objector or/P2toP3-history_object'none' would lead to
<argvalue_id>socurrent_objector orhistory_object. Indeed here it does not seem easily feasible to prepend/What do you think about this ? any other suggestion welcome. I also thought about putting the index numbers in brackets
[P0]or[P2toP3].. but that's a double bracketsmarie commentedon Dec 4, 2020
@plammens you might have an opinion on this last comment ? do not hesitate to jump in :)
arut-grigoryan commentedon Dec 4, 2020
Thanks for your hard work. I don't really understand why the name of the test should change if there is a fixture in the arguments of the case function but it is not parameterized?
#154 (comment) - very good idea i like)
plammens commentedon Dec 4, 2020
Personally I find these aspects of the current naming system confusing:
Referring to two "unpacked" case-parametrized parameters by just joining the names with underscores, e.g., the parameters
inputs,request_dataare shown asinputs_request_data. As you add more and more parameters and fixtures, the test node id becomes increasingly unreadable; it's just a long string of underscores and dashes (which at least to me are not easily distinguishable at first sight). I would make this more explicit by using something like"(inputs, request_data)"or"inputs, request_data"(perhaps without the space) as the argnames id.I'm not sure the argvalue index adds any useful information—if I actually understand what it's doing, which I believe is just indicating the indices of the parameters of the test function being "filled". If we already have the parameter names, the index is just duplicating information, because the parameters of the test function will always have unique names, right? Moreover I find the
P0andP2toP3notation style a bit ugly. I would either remove this entirely (even from the"explicit"style) or change the notation.First, by the looks of things, these are using 0-based indices, correct? If so I find it confusing that they use inclusive ranges (i.e.
P2toP3means indices 2 and 3), since usually ranges in 0-based indexing are half-open, while in 1-based indexing they're closed. I would find2-4more consistent.An alternative notation would be as you said to use brackets. I wouldn't say nested square brackets is too much of a problem, as long as they're balanced :) . Otherwise I would use angle brackets, e.g.
<0>or<2-4>(I don't like the wholePthing). Or maybe we can even use slice notation, which should be intuitive for Python developers:[2:4](in this case it would be really important to use half-open ranges to mimick slice behaviour).I'm also not convinced by using dashes to separate
<argvalue_idx>and<argvalue_id>, as dashes are already used to separate entirely distinct parameters or parameter groups, which again can be visually confusing. As mentioned in the previous bullet I would either remove the argvalue index entirely or use some other notation.If it were to be kept, I would also put the argvalue idx on the other side of the slash: it's providing information about the parameters/argument names, not the particular test case.
(Also, unrelated to this particular issue, but while I'm at it... I don't like the
foo_is_barnotation for fixture unions. Again, here I would introduce some other notation, eg.foo::bar, as iffoowere a "namespace".)My stance can be summarised as "introduce more symbols and alternative notations in order to make each have a(n almost) unique meaning, so that the test node id string can become more compact and readable" 🙂
So, to put everything I've said into a single example, it would look something like this:
In any case what you've proposed, @smarie, is already a great improvement, I would say. These are just my personal itches :)
arut-grigoryan commentedon Dec 11, 2020
Hello. What is the status of the issue?
smarie commentedon Dec 11, 2020
Thanks to both of you for the feedback ! I am finalizing a PR with the following principles:
fixture_unionthe notation appearing in the test ids should be easily identifiable as "this is a fixture union alternative". I prefer not to choose
::(even if it the suggestion is cool!) because this symbol is used already in pytest ids to separate the python file, test class and test function. So I go for/. So afixture_union("a", [b, c])will result in ids["/b", "/c"]by default.idstylein fixture unions:"compact"will lead to["/b", "/c"]. The default style. It makes visible that each parameter is a fixture union alternative and not a "standard" parameter. However it does not overwhelm the user with too long ids"explicit"will lead to["a/b", "a/c"]. This is mostly for debugging, we prepend the name of the union fixture."none"will lead to["b", "c"]. This is or users who do not like the/prefix.<a callable>. For examplestrwill lead to["a/0/b", "a/1/c"], which is an easy way to get the finest debug information on union fixture and its alternatives@parametrizeby default, the user should not feel any special pain/change in parametrize due to
fixture_refbeing present. So by default@parametrizewill have anidstyle=None. So@parametrize("a", [1, fixture_ref(b), 2, 3])will lead to ids["1", "b", "2", "3"]idsworks exactly the same way as in@pytest.mark.parametrize. So passing a callable will receive the argvalues[1, fixture_ref(b), 2, 3].idstylein parametrize is mostly for debug:"none"will lead to["1", "b", "2", "3"]. This is the default behaviour to mimic@pytest.mark.parametrize(users don't care about the fixture union created behind the scenes)"compact"will lead to["/1", "/b", "/P2:4-2", "/P2:4-3"]. This is for debug. It makes visible that there are 3 (and not 4) fixture union alternatives and that the third one is parametrized with two params."explicit"will lead to["a/1", "a/b", "a/P2:4-2", "a/P2:4-3"]. This is for debugging, we prepend the argname. In case of multiple argnames, the prefix will be(a1, a2)as suggested by @plammens<a callable>. For examplestrwill lead to"(argnames,)/<idx>/<argvalues>", which is an easy way to get the finest debug information on union fixture and its alternativesif custom
idsare provided, they will be used to reformat the<argvalue>part of the idstyle. But users will still be able to debug by setting an "explicit" style, even with a custom list/generator/callable of ids.@parametrize_with_casesthis will work entirely similar to
@parametrize:idstyle=Noneby defaultIf a callable is provided as
ids, it will receive the case functions instead of thelazy_valueandfixture_refs created behind the scenes. Users will be able to useget_case_id(f)in this callable to get the case id easily.smarie commentedon Dec 16, 2020
Finally, all tests pass of all versions of python and pytest: https://travis-ci.com/github/smarie/python-pytest-cases/builds/209061483 . I'll merge this PR #155 and release version 2.8.0 in a few minutes.
I opened an issue on pytest because it was quite hard to make the extra useless id from the multi-param fixture disappear in pytest 5.4+ : pytest-dev/pytest#8155
Please let me know of any feedback about the new id system ! Thanks again to both of you for challenging me with this topic, that was the "less interesting" in terms of test running mechanism but was extremely challenging to get done properly including all combinations of custom ids and marks everywhere (and all versions of python and pytest)
smarie commentedon Dec 16, 2020
3.0.0 released (I finally chose to make it a major version bump since it was quite a major refactoring!). Let me know !
arut-grigoryan commentedon Dec 16, 2020
Thank you very much, we are working on)
plammens commentedon Dec 17, 2020
Yes, thank you for all the hard work @smarie !
smarie commentedon Dec 17, 2020
You're welcome. Completing this made me realize that there is still progress to do in terms of simplification, I opened #170 .
Double-thanks to both of you for taking time to try the features and let me know of the issues !