Skip to content

Fix slow marker when conftest is missing. #1259

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

QuLogic
Copy link
Contributor

@QuLogic QuLogic commented Jul 14, 2017

The old fallback does not return anything, breaking anything that might be using the return value, like a marked parametrization.

An example is test_dynamic_workloads_sync in distributed/tests/test_client.py.

The old fallback does not return anything, breaking anything that might
be using the return value, like a marked parametrization.
@QuLogic QuLogic changed the title Fix slow marker when runslow is conftest is missing. Fix slow marker when conftest is missing. Jul 14, 2017
@QuLogic
Copy link
Contributor Author

QuLogic commented Jul 14, 2017

BTW, should conftest be in the PyPI tarball?

@@ -423,8 +423,7 @@ def disconnect_all(addresses, timeout=3):
not pytest.config.getoption("--runslow"),
reason="need --runslow option to run")
except (AttributeError, ValueError):
def slow(*args):
pass
slow = pytest.mark.slow
Copy link
Member

@TomAugspurger TomAugspurger Jul 16, 2017

Choose a reason for hiding this comment

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

This could cause issues with downstream projects who import distributed and run tests with the --strict option. See pandas-dev/pandas#16680. The solution was to register the marker in our setup.cfg like https://github.com/pandas-dev/pandas/pull/16797/files. pandas does distributed our conftest.py, so maybe this change is OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this gets moved to somewhere in tests? Right now this is broken when conftest.py is missing (like in the PyPI tarballs.)

@mrocklin
Copy link
Member

Would this all be resolved if we distributed the conftest.py file?

@TomAugspurger
Copy link
Member

TomAugspurger commented Jul 17, 2017

That's what pandas ended up doing. We had to since we have pd.test() to run the test suite.

@QuLogic
Copy link
Contributor Author

QuLogic commented Jul 18, 2017

But does Pandas add an extra option? In Matplotlib, we couldn't add conftest.py in both packages and the top-level (i.e., distributed, but not part of a package), so we didn't bother with adding any options. This might not affect distributed though since it doesn't need both.

@mrocklin
Copy link
Member

This PR has gone stale. Closing for now.

@mrocklin mrocklin closed this Apr 15, 2019
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.

3 participants