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

Pass positional arguments to specific sessions #391

Closed
rmorshea opened this issue Feb 26, 2021 · 7 comments · Fixed by #397
Closed

Pass positional arguments to specific sessions #391

rmorshea opened this issue Feb 26, 2021 · 7 comments · Fixed by #397

Comments

@rmorshea
Copy link
Contributor

rmorshea commented Feb 26, 2021

The Problem

One issue with the way positional arguments work right now is that they are shared between all sessions.

  • For some use cases this makes sense: a --debug flag could be a piece of global information that all sessions would want to know about.
  • In others though it does not: I might want to pass extra positional arguments through to a specific session.run command that do not concern the other sessions.

Here's a more concrete example...

Imagine that I have two sessions run_pytest and another called build_sphinx_docs. I want to pass extra arguments to pytest when running tests in the first session, and I want to pass arguments to sphinx when building the documentation.

nox -- --debug --no-cov --headless --doctest
#      ^global ^pytest  ^pytest    ^sphinx

Presently if I want to do this I have to devise a custom solution for parsing which arguments belong to which session. It would be great if I could specify which arguments belong to which session. Something like:

nox -- --debug run_pytest[--no-cov,--headless] build_sphinx_docs[--doctest]

My Current Solution

In my own projects I created a simple utility to solve the problem described above. Admittedly though my implementation isn't perfect though and could probably use some refinement to make it work in most or all use cases. Here it is though:

import re
from typing import List
from nox import Session

posargs_pattern = re.compile(r"^(\w+)\[(.+)\]$")

def get_posargs(name: str, session: Session) -> List[str]:
    """Find named positional arguments

    Positional args of the form `name[arg1,arg2]` will be parsed as ['arg1', 'arg2'] if
    the given `name` matches. Any args not matching that pattern will be added to the
    list of args as well (these are considered global). Thus the following where `name`
    is `session_1`...

    --param session_1[arg1,arg2] session_2[arg3,arg4]

    ...would produce ['--param', 'arg1', 'arg2']
    """
    collected_args: List[str] = []
    for arg in session.posargs:
        match = posargs_pattern.match(arg)
        if match is not None:
            found_name, found_args = match.groups()
            if name == found_name:
                collected_args.extend(map(str.strip, found_args.split(",")))
        else:
            collected_args.append(arg)
    return collected_args

And would be used in a noxfile.py like this:

@nox.session
def run_tests(session: Session) -> None:
    """Run the Python-based test suite"""
    session.install("pytest")
    session.install(".")
    extra_posargs = get_posargs("pytest", session)  # <-- only grabs args for pytest
    session.run("pytest", "tests", *extra_posargs)

With usage at the command line looking a bit like this:

nox -- pytest[--maxfail=2,--tb=long] --this-param-is-ignored this_is_too[--flag]

One thing I like about the way this works is that I can drop the specificity if I'm only running one session:

nox -s run_pytest -- --maxfail=2 --tb=long

These positional arguments are now "global", but because I'm only running one session that doesn't matter, and get_posargs gracefully handles that.

@theacodes
Copy link
Collaborator

I'm kinda neutral on this. I don't want to complicate Nox's invocations any further, but I can also see the utility in this. I also pathologically avoid long command invocations since it's easy to forget them.

I think an alternative I might prefer is to have a session that runs the others with the args:

@nox.session
def something(session):
     session.notify("run_pytest", "--no-cov", "--headless")
     session.notify("build_sphinx_docs", "--doctest")

We'd have to modify notify to take posargs to pass along.

@rmorshea
Copy link
Contributor Author

rmorshea commented Feb 27, 2021

That seems reasonable. Like you said, most people aren't going to remember commands with many arguments. However when it's required, those commands are usually codified in a dedicated script. This notify idea mimics that rather nicely and would solve my use case.

@theacodes
Copy link
Collaborator

theacodes commented Feb 27, 2021 via email

@cjolowicz
Copy link
Collaborator

I have a similar, yet different use case, where I'd like to avoid passing
session arguments to a notified session.

What would you think about giving session.notify a keyword argument posargs,
where None (the default) is the current behavior, and anything else would set
session.posargs as specified? Then we could pass [] or () to prevent the
notified session from seeing arguments that are not intended for it. You could
also use this to tack additional arguments onto session.posargs, or filter it.

FWIW, the code below shows my use case and current workaround. There are two
sessions tests and coverage, with tests notifying coverage. Both accept
session arguments, forwarded to pytest and Coverage.py, respectively. Currently,
when tests is invoked with extra arguments for pytest (say, --verbose), the
notified coverage session has to be careful not to pass these to Coverage.py as
well. The only workaround I could find was having the coverage session check
that it's the only one requested by the user. But this uses an internal Nox API
(session._runner.manifest), so it's not a sustainable solution.

@nox.session(python=python_versions)
def tests(session):
    session.install(".", "coverage[toml]", "pytest")
    try:
        session.run("coverage", "run", "--parallel", "-m", "pytest", *session.posargs)
    finally:
        if session.interactive:
            session.notify("coverage")

@nox.session
def coverage(session):
    # Do not use session.posargs unless this is the only session.
    has_args = session.posargs and len(session._runner.manifest) == 1
    args = session.posargs if has_args else ["report"]

    session.install("coverage[toml]")

    if not has_args and any(Path().glob(".coverage.*")):
        session.run("coverage", "combine")

    session.run("coverage", *args)

Using what's proposed above, this could be rewritten like this:

@nox.session(python=python_versions)
def tests(session):
    ...
    finally:
        if session.interactive:
            session.notify("coverage", posargs=[])

@nox.session
def coverage(session):
    args = session.posargs if session.posargs else ["report"]
    ...

@theacodes
Copy link
Collaborator

Sounds good to me.

@rmorshea
Copy link
Contributor Author

rmorshea commented Feb 28, 2021

@theacodes I'm not seeing how posargs would get passed from notify to the session. At the moment, it seems like posargs are fundamentally global: https://github.com/theacodes/nox/blob/a078d3cc82c09a3af068d7c9d230364ba5adefbb/nox/sessions.py#L154

@cjolowicz
Copy link
Collaborator

I'm not seeing how posargs would get passed from notify to the session.

Would it make sense to give SessionRunner an attribute notify_posargs
(initially None) and set that from Manifest.notify? Then you could return it
from Session.posargs if it's not None, and fall back to current behavior
otherwise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants