autotest / autotest-docker

Autotest client test containing functional & integration subtests for the Docker project
Other
25 stars 30 forks source link

Make 'name_prefix' into a default option #331

Closed cevich closed 9 years ago

cevich commented 10 years ago

When empty, Subtest and SubSubtest __init__() can set it == the class name.

ldoktor commented 9 years ago

How about a bit different approach. We can modify the containers.get_unique_name

        if prefix is None:
            prefix = self.subtest.__class__.__name__
        if suffix is None:
            suffix = ""

And in tests call it with config.get('name_prefix')? In case it's undefined, it'll get the Subtest's name. The drawback is that neither containers nor images accept SubSubtests, so we'd get only Subtest's name. (rmi-240d)

Additionally why do we actually need to set the name prefix? Usually we don't care. So the step 2 would be to remove config.get('name_prefix') from most places and use just containers.get_unique_name(). We'd only keep the custom name on places, where it makes sense, like for example when we have 2 containers, one as server, other as client. Then the names shouldn't be in config, but rather in the test itself as it is part of the test structure. (server-3a20)

@cevich, @jzupka what do you think?

PS: It's actually a good idea to use class name, we might also use them always and append the custom name afterwards. This way we'd know in which test this container was created (rmi-server-1a2c)

cevich commented 9 years ago

I like it, but having sub-subtest names in prefixes would be super handy. You can derive the name really easy with os.path.basename(self.subtest.config_section). Should work with both subtests and sub-subtests. If you want to detect the difference, check for subsubtests in self.subtest.config.

Additionally why do we actually need to set the name prefix?

Yeah, you're right, we don't need it. I agree, totally get rid of name_prefix configuration everywhere is the answer. I can't see a reason why we really ever need to set it or worry about it from a defaults/global perspective. If we do, it can be hard-coded to the containers.get_unique_name() call.

Actually, probably the same thinking applies to dockerimages.get_unique_name() as well :wink:

ldoktor commented 9 years ago

yep, I actually changed it in dockerimages without asking :smile:. Anyway I'm not sure I understand the way you used to get subsubtest name. The problem is, that booth containers and dockerimages accept only Subtest and not Subbase. Thus when you create it, you provide only self.parent_subtest. Correct me if I'm wrong, but I can't get subsubtest name from parent subtest (because it's parent subtest already...).

Anyway I guess we can live with Subtest name for now and when we make containers and dockerimages accept subbase it will magically start logging the correct names. What do you think?

cevich commented 9 years ago

Anyway I guess we can live with Subtest name for now and when we make containers and dockerimages accept subbase it will magically start logging the correct names. What do you think?

:confounded: really?!?!?! Whoa! Looks like I updated images but never containers. It's just a documentation change really. Okay, #354, done. Sorry, thought I fixed that a long while ago.

I'm not sure I understand the way you used to get subsubtest name

All SubBase derived instances use the config_section to hold both the configuration section name and the subtest or sub-subtest name, they have to be identical. It's guaranteed to always be there by __init__().

Further, sub-subtests should/will never contain a configuration option named 'subsubtests', only the parent subtest will. Another option is to check for existence of parent_subtest attribute, then you know you've got a sub-subtest.

Though, now that I think of it and IIRC, isinstance() will also check sub-classes (i.e. so SubSubtestCaller should satisfy isinstance(subsubcaller, Subtest). That's probably the safer way, IIRC.

cevich commented 9 years ago

Resolved by #357