Skip to content
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

Don't create DSession in collect-only mode #232

Merged
merged 4 commits into from
Oct 5, 2017

Conversation

verdesmarald
Copy link
Contributor

@verdesmarald verdesmarald commented Sep 4, 2017

Fixes #5


Thanks for submitting a PR, your contribution is really appreciated!

Here's a quick checklist that should be present in PRs:

  • Make sure to include reasonable tests for your change if necessary

  • We use towncrier for changelog management, so please add a news file into the changelog folder following these guidelines:

    • Name it $issue_id.$type for example 588.bugfix;

    • If you don't have an issue_id change it to the PR id after creating it

    • Ensure type is one of removal, feature, bugfix, vendor, doc or trivial

    • Make sure to use full sentences with correct case and punctuation, for example:

      Fix issue with non-ascii contents in doctest text files.
      

Copy link
Member

@RonnyPfannschmidt RonnyPfannschmidt left a comment

Choose a reason for hiding this comment

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

this is a good enhancement 👍 , thanks for taking the time to make it

please add a test to prevent regression

xdist/plugin.py Outdated
config.pluginmanager.register(session, "dsession")
tr = config.pluginmanager.getplugin("terminalreporter")
tr.showfspath = False
if config.getoption("boxed"):
Copy link
Member

Choose a reason for hiding this comment

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

this "incorrectly" disaables the aliasing of boxed to forked,

it just doesn't show because collectonly doesnt run any tests

Re-enable parsing of the boxed option with collect-only
@verdesmarald
Copy link
Contributor Author

I've added an acceptance test and some unit tests in test_plugin. I've also re-enable the boxed option with collect-only even though it has no effect since no tests are run.

@RonnyPfannschmidt
Copy link
Member

well done will merge after pass

@RonnyPfannschmidt
Copy link
Member

flake8 strikes again ^^

@verdesmarald
Copy link
Contributor Author

Oops, sorry!

@RonnyPfannschmidt
Copy link
Member

don't worry, i suffer the same fate way to regular ^^
thats why we have that ci check to begin with

@nicoddemus nicoddemus merged commit 95b660e into pytest-dev:master Oct 5, 2017
@@ -44,6 +44,36 @@ def test_auto_detect_cpus(testdir, monkeypatch):
assert config.getoption('numprocesses') == 99


def test_boxed_with_collect_only(testdir):
Copy link
Member

Choose a reason for hiding this comment

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

i would consider that on unnecessary - forked/boxed got removed from xdist

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.

Poor interaction between -n# and --collect-only
3 participants