Skip to content

Conversation

BeyondEvil
Copy link
Contributor

No description provided.

try:
markers = dict()
for each in node.iter_markers(name):
markers.update(each.kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand it, this will find the capabilities markers applied to the various scopes and methods in use. The capabilities from each marker will be updated by the next. Whilst this it probably what we should have been doing all along, it could cause some confusion and potential breakage to existing users. I would suggest adding debug level logging for each marker found with as much context as we can. That way, a user can turn on debug logging and understand where/why any capabilities are being overwritten.

Copy link
Contributor Author

@BeyondEvil BeyondEvil Aug 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some testing with module and function level markers, and that code behaves just as get_markers() did. That is, if there's a conflict between module and function, the module wins.

With that said, I can see if I can add more debugging.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh so we don't iterate through them? That's interesting.. then what are we iterating through? Multiple markers with the same name on a single object?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So iter_markers() returns a generator containing Mark objects that looks like so: Mark(name='capabilities', args=(), kwargs={'one': 1, 'two': 21})

markers.update(each.kwargs)
return markers
except AttributeError: # backwards-compat
marker = node.get_marker(name)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you mention when we could remove this? Once we require a certain minimum version of pytest, we can simplify this code.

return capabilities


def _get_marker(node, name):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't returning the markers, but a dictionary of the marker's keyword arguments. I appreciate your desire to make this a generic function, but I think in this case we could just name it get_capabilities_from_markers. We could also simplify to capabilities.update(get_capabilities_from_markers(request.node)) as the return value will always be a dictionary so we don't need the if x is not None block.

ignore:--firefox-\w+ has been deprecated:DeprecationWarning
ignore:MarkInfo:DeprecationWarning:pytest_selenium.drivers.firefox:88
""")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! What do we need to do to enable failure on warnings?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you want it to fail on all warnings, not just DeprecationWarnings?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's discuss in a new issue/pull request. I'm thinking just deprecation warnings, but also interested to know what other warnings we have.

@davehunt davehunt merged commit 25376a9 into pytest-dev:master Aug 23, 2018
@BeyondEvil BeyondEvil deleted the handle_deprecations branch August 23, 2018 14:38
BeyondEvil added a commit to BeyondEvil/pytest-selenium that referenced this pull request Sep 16, 2018
…dev#183)

Release 1.14.0 (pytest-dev#184)

pre commit config

fixes by BLACK and badge in readme

Updates based on comments

Fix deprecation and add doc and test

Fix tests that fail locally

requested changes

run tests in parallel

pre commit config (pytest-dev#185)

* pre commit config

* fixes by BLACK and badge in readme

* Updates based on comments

* Fix deprecation and add doc and test

* Fix tests that fail locally

* requested changes

Move credentials to capabilities

mark failing test as xfail

Run tests in parallel using pytest-xdist (pytest-dev#188)

Also marking Firefox extension test as expected to fail due to current incompatibilities with Selenium and web extensions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants