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

Add transient arg for pytest-xdist parallelization #54

Closed
wants to merge 1 commit into from
Closed

Add transient arg for pytest-xdist parallelization #54

wants to merge 1 commit into from

Conversation

cvdub
Copy link
Contributor

@cvdub cvdub commented Dec 9, 2021

This PR adds rudimentary support for the pytest-xdist -n flag.

@wbolster
Copy link
Owner

wbolster commented Dec 9, 2021

thanks, but i think this won't work out of the box because it depends on pytest plugins?

not sure how to handle that gracefully. thoughts?

similar things apply for for coverage flags (pytest-cov)

@wbolster wbolster added enhancement New feature or request needs info labels Dec 9, 2021
@wbolster wbolster self-assigned this Dec 9, 2021
@pkryger
Copy link
Contributor

pkryger commented Dec 9, 2021

One option could be to query for installed plugins and display their respective options. Having said that, I found that pip is borderline unacceptable slow on my machine to do it on each popup (cache it in buffer local, but how to refresh?). Anyway, did that dirty bit of code (requires json) that may be of help

(defun pytest--plugins (packages)
  "Find information about installed PACKAGES.

Return an alist that elements are cons of package from PACKAGES
and the installed version (or nil if not installed)."
  (let ((installed-packages
         (with-temp-buffer
           (shell-command  "pip list --format=json" (current-buffer))
           (goto-char (point-min))
           (json-read))))
    (seq-map
     (lambda (plugin)
       (cons plugin
             (seq-some
              (lambda (pkg-info)
                (let-alist pkg-info
                  (when (string= .name plugin)
                    .version)))
              installed-packages)))
     packages)))

(pytest--plugins '("pytest-xdist" "pytest-cov" "pytest-foo"))
-> (("pytest-xdist" . "2.4.0") ("pytest-cov" . "2.12.1") ("pytest-foo"))

Edit: now when I think of it, it doesn't account that pytest may be called from "somewhere". I.e., /my/special/path/to/pytest-wrapper.sh that will call pytest installed somewhere with some plugins... a simple pip called shell-command won't cut it

@cvdub
Copy link
Contributor Author

cvdub commented Dec 9, 2021

Good call on only adding this if the plugins are installed.

pytest actually returns plugin information if you pass -V/--version twice.

This function returns t if a plugin is installed:

(defun python-pytest--plugin-installed-p (plugin-name)
  "Returns t if PLUGIN-NAME is installed."
  (let* ((default-directory (python-pytest--project-root))
         (pytest-version-string (shell-command-to-string (concat python-pytest-executable " -V -V"))))
    (and (string-match-p plugin-name pytest-version-string) t)))

But I'm not sure the best way to conditionally include the plugin flags to the transient. Any ideas there?

@wbolster
Copy link
Owner

wbolster commented Dec 13, 2021

tbh i'm not a fan of dynamic plugin inspection

  • potentially slows down
  • caching may help, but they can go stale
  • requires venv activation (direnv ftw, btw) for correct behaviour

both pytest-cov and pytest-xdist are widely used; are there any downsides to just including some flags (maybe with a hint in the section title) in the popup?

@pkryger
Copy link
Contributor

pkryger commented Dec 14, 2021

tbh i'm not a fan of dynamic plugin inspection [...]

To add to that list, I'm not sure neither how to deal with it (redefine python-pytest-dispatch each time it's invoked?) nor how it would interfere with other transient's features, i.e., restoring a saved state when option is not available.

both pytest-cov and pytest-xdist are widely used; are there any downside to just including some flags (maybe with a hint in the section title) in the popup?

While I can see the former is being used in a few projects I'm working with these days, I cannot see the latter. Calling pytest -n 2 yields:

ERROR: usage: pytest [options] [file_or_dir] [file_or_dir] [...]
pytest: error: unrecognized arguments: -n
  inifile: /project/dir/setup.cfg
  rootdir: /project/dir

While I don't know yet how to achieve that and I acknowledge that would require some work, perhaps it's worth investigating filtering out unknown options right before calling pytest? It would add some time (pytest -V -V is a few hundred ms in one of my projects), but could result in more desirable UX. The filter out could be after a question or a warning, that is suppressible with a customization option. @cvdub @wbolster what do you think?

@wbolster wbolster closed this in e491a98 Dec 14, 2021
@wbolster
Copy link
Owner

wbolster commented Dec 14, 2021

i've added a variation of this pr (i kept @cvdub as main author) in e491a98.

it adds these dispatch options:

  • -n which expands to --numprocesses=... with a shorter list of options
  • -f which expands to --looponfail (toggle)

@cvdub and @pkryger sounds good?

@wbolster
Copy link
Owner

re pytest-cov: it's widely used but thinking of it, it requires proper config in pyproject.toml (or pytest.ini) anyway to be useful, which basically leaves a --no-cov option for toggling it off. i don't think that's worth it

@pkryger
Copy link
Contributor

pkryger commented Dec 14, 2021

With all that has been said, I'm loosing my appreciation for dynamic plugin discovery. At least for now. Perhaps in a future we may want to revisit.

What's been done in e491a98 - LGTM

@cvdub cvdub deleted the pytest-xdist branch December 14, 2021 22:59
@cvdub
Copy link
Contributor Author

cvdub commented Dec 14, 2021

Thanks for your work on this @wbolster!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs info
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants