feat: allow defining a python version list for GHA action (alt)

[mayeut]

This is another way to implement the feature discussed in #601.

It hopefully addresses any shortcomings in the implementation except for the yaml list which is still not possible but has been replaced with a simple enough comma-separated list.

It now supports any python version supported by actions/setup-python@v3 with up-to 20 versions installed.
The python validation/helper script which is currently inlined in the action might benefit from being a standalone script allowing for proper testing. I can do this if maintainers want to pursue this further.

One thing discovered while working on this is that mixing actions/setup-python & wntrblm/nox actions might not be that safe.
e.g. let's say that for some reason that I need pypy-3.9-nightly because of a bug in pypy-3.9:

    # this breaks cpython3.9 testing (overridden by pypy-3.9-nightly)
    - uses: wntrblm/nox
    - uses: actions/setup-python@v3
      with:
        python-version: "pypy-3.9-nightly"

    # this is just no-op (pypy-3.9-nightly overridden by wntrblm/nox)
    - uses: actions/setup-python@v3
      with:
        python-version: "pypy-3.9-nightly"
    - uses: wntrblm/nox

    # we currently need to do this
    - uses: wntrblm/nox
    - uses: actions/setup-python@v3
      with:
        python-version: "pypy-3.9-nightly"
    - uses: actions/setup-python@v3
      with:
        python-version: "3.9"

    # with this PR 
    - uses: wntrblm/nox
      with:
        python-versions: "3.7, 3.8, 3.9, 3.10, pypy-3.8, pypy-3.9-nightly"

closes #601

[FollowTheProcess]

See review comments. Am I correct in thinking that the additional validation logic here is purely a result of avoiding the "['3.6', '3.7', '3.8'...]" syntax?

If so I don't think the increased complexity is worth it and I'd actually be happier with a string list syntax. Sorry that I've flip flopped on this but I didn't appreciate how much additional complexity would be required

[FollowTheProcess]

I think this would definitely benefit from being a standalone script, and I'd ideally like to see a decent number of test cases to ensure it handles weird edges, there's plenty of opportunity in here to fail in an ugly way (e.g. the indexing).

However if this is only required because of supporting the slightly nicer list syntax then I'd argue the better approach is to simply live with the string list syntax.

[mayeut]

I think this would definitely benefit from being a standalone script

done

and I'd ideally like to see a decent number of test cases to ensure it handles weird edges, there's plenty of opportunity in here to fail in an ugly way (e.g. the indexing).

will do if we decide to go further on this.

However if this is only required because of supporting the slightly nicer list syntax then I'd argue the better approach is to simply live with the string list syntax.

that's not for a slightly nicer list syntax, more on that later.

[FollowTheProcess]

Apologies for my flip flopping on this but if this level of additional work is what's required to avoid the "['3.7', '3.8', '3.9'...]" syntax then I don't think it's worth it and I'd be happier to live with the string list. At least with the former we don't have to have a custom validator?

[mayeut]

thanks for the review, I'm a bit short on time until this week-end so I will address most remarks then.
In the meantime, #606 is probably easier to review and included here (#609 (comment) is applicable to #606 so maybe we should start there ?)

[mayeut]

Am I correct in thinking that the additional validation logic here is purely a result of avoiding the "['3.6', '3.7', '3.8'...]" syntax?

If so I don't think the increased complexity is worth it and I'd actually be happier with a string list syntax. Sorry that I've flip flopped on this but I didn't appreciate how much additional complexity would be required

The validation/helper logic is not only there to avoid the "['3.6', '3.7', '3.8'...]" syntax. That's only a small aspect of what's being achieved in this PR.

The main goal with this alternative to the implementation of #601 is for the wntrblm/nox action to be able to install any python version that can be installed with actions/setup-python correctly. The list of python version that can be installed with the action in #601 is hardcoded meaning it will require some updates every now & then (e.g. when python 3.11 is released or python 3.7 EOL, ...).

The implementation here comes with a small increase in complexity but is in my opinion more usable and I feel that there will be less maintenance on the action in the long run.

[FollowTheProcess]

The implementation here comes with a small increase in complexity but is in my opinion more usable and I feel that there will be less maintenance on the action in the long run.

So this one is intended to replace #601 ?

[mayeut]

So this one is intended to replace #601 ?

short answer: yes.
longer answer: I'm keeping #601 opened for now if maintainers think this added complexity is really too much to handle. #601 might be an acceptable middle ground.

[FollowTheProcess]

Hi @mayeut sorry for the delay, and apologies I know I'm not making this easy for you 😂 This is looking good, I've just left a few comments re. supporting EOL'd pythons.

I think we should stick to >3.7 and if anyone wants to set up Nox in EOL'd pythons then it's on them to do it manually. Doing this should simplify the testing somewhat as AFAICT we wouldn't need the concept of default_tests and all_tests?

Also means we can include the helper in the linting suite.

Only other thing is ideally I'd like to see some tests on the helper, doesn't have to be extensive just verifying a number of versions and asserting it's doing the right thing for each and handling any weirdness.

This is shaping up nicely, thanks for all your efforts so far!

[mayeut]

I think we should stick to >3.7 and if anyone wants to set up Nox in EOL'd pythons then it's on them to do it manually. Doing this should simplify the testing somewhat as AFAICT we wouldn't need the concept of default_tests and all_tests?

Also means we can include the helper in the linting suite.

As mentioned in an earlier comment, one has nothing to do with the other. The helper shall be Python 3.6 compatible if the ubuntu 18.04 GitHub Action runner is to be supported because it runs Python 3.6, supported by Canonical for security updates rather than the CPython core team.

The fact that Nox dropped support for Python < 3.7 is one thing and that's completely fine however this only covers the Python version used to install & run Nox. If running a session with EOL/dev python is still supported (& it is as far as I know), then supporting those in the action & this test does still make sense IMHO.

Only other thing is ideally I'd like to see some tests on the helper, doesn't have to be extensive just verifying a number of versions and asserting it's doing the right thing for each and handling any weirdness.

will do EDIT: done.

[FollowTheProcess]

Sorry for the long delay, been a bit of a busy period! This is looking really good and thanks @mayeut for all the effort you've put in so far. It would be good to get a couple other maintainers to weigh in with any thoughts/comments (@wntrblm/nox) but this looks good to me 👍🏻

[theacodes]

LGTM

[mayeut]

Thanks for the feedback. I rebased on main to fix the conflict.

[FollowTheProcess]

Thanks @mayeut 🎉