Make passing plugins as str objects a more obvious failure

[nicoddemus]

See #855

CC @xmo-odoo

[xmo-odoo]

Maybe warn in case of unicode strings as well? Not sure how pytest handles the 2/3 split though.

[flub]

Using py.builtin._basestring IIRC

[nicoddemus]

Done, thanks! 😄

[flub]

I think it would be better to do this fix in pluggy rather then in py.test to be honest.

Also, is something like not isinstance(plugin, (types.ModuleType, types.ClassType, types.InstanceType)) not more correct for this check?

[flub]

Hum, maybe the isinstance I suggest is not really in the spirit of ducktyping. And hence the generic approach in pluggy not so great. Furthermore a plugin could provide no hooks but only some fixtures in which case the generic approach in pluggy is also not so great. So maybe my previous comment is wrong and this PR is the right approach anyway.

[xmo-odoo]

Also, is something like not isinstance(plugin, (types.ModuleType, types.ClassType, types.InstanceType)) not more correct for this check?

Well you'd also need to add object to it as any arbitrary instance could be a plugin, but then you face the issue that isinstance("foo", object) is True.

Furthermore a plugin could provide no hooks but only some fixtures in which case the generic approach in pluggy is also not so great.

It just happens that my original use case was indeed a plugin which only provided fixtures, no hooks. And my feature request was about excluding strings: I had thought that since -p takes a "string" designator for plugins it could also work for the plugins argument. It is an ID10T error, but one which I think has at least a bit of logic threaded through it.

[RonnyPfannschmidt]

random thought, could we just consider strings in the same manner we consider the -p option ?

[xmo-odoo]

That would certainly be an alternative.

[nicoddemus]

Good idea @RonnyPfannschmidt, updated the PR to interpret strings as module names instead. 😄

[RonnyPfannschmidt]

+1