markers - a rough path into the future (and object based markers)

[RonnyPfannschmidt]

this is a rough sketch of alternative to #5418 and builds upon marks - proposals for a new api and a path forward

as a starting point we should elevate actual markers from the need to register a name with us, they should use the solid python name-spacing systems using modules and type names

so we would switch the marker implementation from something like

Mark("skipif", (), {"condition": ..., "reason": ...}) to ConditionalSkip(condition=..., reason=...)

after freeing the actual marker registration up, we can then follow up with registering the names of marker constructors something like the following in a conftest.py/plugin

# use a hook to create 0-N object based markers for a classical declaration
# should warn if no implementation triggered and no element was in the ini-lines for markers
def pytest_make_marker(config,  name, args, kwargs):
   ...
   return MyMarker(*args, **kwargs)

# used to map well known mark names to their implementation,
# more than one one module  may implement the same marker, 
# implemented by the pytest core `pytest_make_marker` hook
@pytest.markspec
def skip_if(condition, reason=None) -> Option[ConditionalSkip]:
  return ConditionalSkip(condition, reason)

# make a marker known that acts as a symbol, perhaps this should use a different function name
xfail = pytest.markspec.symbol("xfail") 

lookup should happen in terms of types for the new style markers the name based lookup need to be expanded in terms of looking up the types that natch a name

[nicoddemus]

Hi @RonnyPfannschmidt,

Thanks for the write up.

A few questions:

1) It is not clear to me if you are proposing pytest_make_marker and @pytest.markspec as alternatives, or proposing to have both at the same time. Can you clarify that?

2) Regarding xfail symbol, does that pytest.markspec.symbol declaration means users will use it as @pytest.mark.xfail or @pytest.mark(xfail)?

---

I definitely agree that object-based markers as in:

from pytest_timeout.marks import timeout

@pytest.mark(timeout(10))
def test():
    ...

Have the clear advantage of providing a consolidated namespace and clear signature that can easily be looked-up and validated by linters, but I worry that now we have two very distinct ways of doing the same thing, and this duality might be confusing/seem complicated at first. Any thoughts on that?

Another concern is that @pytest.mark(timeout(10)) and @pytest.mark.timeout(10) are too close to each other, again possibly being a cause for confusion. Perhaps we should define a different way to apply class-based marks than reusing @pytest.mark?

[nicoddemus]

Btw should we send an e-mail to the mailing list so others can join this discussion? Many people there don't follow GitHub discussions closely.

[RonnyPfannschmidt]

1. markspec would be implemented in terms of make_marker
2. users could use both, the preference would be via pytest.mark.foo instead of importing the symbol

[nicoddemus]

Thanks for the reply.

Question 2) answered, thanks.

Back to 1), still not clear to me why would we need the pytest_make_marker hook at all.

Without understanding the need for the hook, I believe this could work:

@pytest.markspec
def timeout(value, *, method=None):
  return TimeoutMark(value=value, method=method)

# used as
from pytest_timeout.mark import timeout
@pytest.mark(timeout(10))
def test_foo():
    pass

And to keep the previous @pytest.mark.foo syntax to work, we could add an option to markspec to auto-register it:

@pytest.markspec(register=True)
def skip_if(condition, *, reason=None):
  return ConditionalSkip(condition, reason=reason)

# used as
@pytest.mark.skipif('sys.platform == "win32"')
def test_foo():
    pass

?

Also when you have the time, would like to know if you have the same concerns that I wrote at the bottom of my post. 👍

[RonnyPfannschmidt]

markspecs always register else a function would do

the concern is kinda valid, but practically, if you use markspecs, you use pytest.mark.foo if you use plain classes, you use pytest.mark(SomethingFun(....))

fun will happen when we mix things like

@markspec
def skipif(condition: MarkCondition, reaon: Option[str] =None):
  ...

@markspec
def skipif(bug: IssueIdentifier):
...

[nicoddemus]

I see, this clarifies things a bit, but I still don't know what's the use for the pytest_make_marker hook then. 🤔

[RonnyPfannschmidt]

@nicoddemus i was thinking of disambiguating markers for example, but it seems it currently lacks the parameters for that, so that hook is probably a bad idea

[nicoddemus]

Thanks.

I would like to note that we can introduce markspec are independent from class-based markers, they don't need to get implemented at the same time (at least seems to me that should be possible).

[nicoddemus]

Another thought:

@pytest.markspec
def skipif(condition, reason=None) -> Option[ConditionalSkip]:
  return ConditionalSkip(condition, reason)

With the above declaration, it means that node.get_closest_marker('skipif') will now return a ConditionalSkip object rather than a Mark. This might be a problem if someone access mark.args or mark.kwargs unconditionally:

for m in node.iter_markers():
    print(m.args)

But I think that's rare and should not really be a roadblock.

[RonnyPfannschmidt]

actually, we should return Marks there, the api to get objects should be different

[nicoddemus]

Hmm yet another API? Wouldn't that bring even more confusion users? 🤔

[RonnyPfannschmidt]

its a different thing with a different behaviour, its absolutely unacceptable to dump it into a old api

[RonnyPfannschmidt]

note that we dont need a new function name,

iter_markers("name") -> Mark objects

iter_markers(named_by="name") -> objects that got returned from registred markers

iter_markers(instance_of=type) -> objects that are instances of the named types

named_by and instance_of may be combined in a call and should compound in effect/selectiveness