nox: Change default behavior for `--error-on-missing-interpreters`

Is it possible to improve --error-on-missing-interpreters with “principle of least surprise” in mind? We don’t want onerous failures during local dev, but the two changes are proposed:

Mention the --error-on-missing-interpreters flag in the error / warning / success message if an interpreter was skipped
Change --error-on-missing-interpreters to default ON (instead of OFF) if we’ve detected the code is in CI (CI=true) or in an automated environment (sys.stdout.isatty()==False)

Original Content (Original title: Test suite skipped with error "Python interpreter 3.8 not found.)

Describe the bug

The desired Python version was not available, the test suite is skipped entirely.

2020-08-21T17:46:46.1946078Z ##[group]Run nox
2020-08-21T17:46:46.1946291Z [36;1mnox[0m
2020-08-21T17:46:46.1990422Z shell: /bin/bash -e {0}
2020-08-21T17:46:46.1990565Z env:
2020-08-21T17:46:46.1990705Z   pythonLocation: /opt/hostedtoolcache/Python/3.7.8/x64
2020-08-21T17:46:46.1990822Z ##[endgroup]
2020-08-21T17:46:46.2954365Z Running session tests-3.8
2020-08-21T17:46:46.3002157Z Session tests-3.8 skipped: Python interpreter 3.8 not found.

How to reproduce

import nox

@nox.session(python=["3.7"])
def tests(session):
    """Run pytest suite"""
    session.create_tmp()
    session.install("-r", "requirements-test.txt")
    tests = session.posargs or ["tests/"]
    session.run("pytest", *tests)

# TODO: add lint, type checking, etc.

$ cat .github/workflows/data_prep_tools_test.yml 
# This workflow will install Python dependencies, run tests.
# For more information see: https://help.github.com/actions/language-and-framework-guides/using-python-with-github-actions

name: test

on:
  push:
    branches: [ master ]
  pull_request:
    branches: [ master ]
    
defaults:
  run:
    working-directory: tools

jobs:
  build:

    runs-on: ubuntu-latest
    strategy:
      matrix:
        python-version: [3.7]

    steps:
    - uses: actions/checkout@v2
    - name: Set up Python ${{ matrix.python-version }}
      uses: actions/setup-python@v2
      with:
        python-version: ${{ matrix.python-version }}
    - name: Install dependencies
      run: |
        pip install nox
    - name: Test with nox
      run: |
        nox

Expected behavior

non-zero exit code

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 17

Most upvoted comments

+1 for option 3:

3. Make CI=true change the default error-on-missing-interpreters to True (that one is listed above) This is a really nice convention and not specific to a single CI vendor.

Additionally, it would be good to add the warning described here:

Prints out a warning when interpreters are skipped saying that you might want to use --error-on-missing-interpreters and that this will fail by default on CI systems.

I’m -0.5 for option 1:

Make it an error if no interpreter at all is found - at least one must match.

it’s a larger breaking change as it affects local runs
it means that --[no-]error-on-missing-interpreters is no longer the single source of truth for what happens if an interpreter is not found.
option 3 is simpler to implement and solves the use case just as well, if not better
there are tricky edge cases: session.notify, @nox.parametrize with a matrix including Python versions, and generally the many ways of specifying interpreters and sessions

cjolowicz on Feb 5, 2022

I’ve been tripped up by this too, and actually so has Nox itself: https://github.com/theacodes/nox/pull/475.

IMO the average user shouldn’t have to care about this which means we can’t add pain to local runs whilst doing the correct thing on CI, I think the cleanest way of doing this is checking for the CI env variable and making sure local skipped sessions print out a nice helpful warning saying they will not skip on CI and will fail instead if the interpreter is not found as @theacodes suggests.

Maybe say if you want to emulate behaviour on CI, set CI in your environment or run with --error-on-missing-intepreters.

I appreciate the concern that this is different behaviour based on a somewhat hidden environment state but ultimately, CI is a different environment and in this case I think it makes sense that we behave differently.

I’m also not against the --non-interactive based solution, I appreciate @henryiii’s concern about paging but I suspect that not a huge amount of people are piping their nox runs to a pager? I could be wrong though!

Either way, something we could do quickly to address this is document @henryiii’s workaround:

if os.environ.get("CI", None):
    nox.options.error_on_missing_interpreters = True

We could discuss forever, I think the best thing to do is just take a crack at a solution and get it into PR review for us all to discuss a bit more concretely. I’ll try and have a go over the next week or so 👍🏻

FollowTheProcess on Feb 5, 2022