Make matrix run all cases of the python-deps tests

[robertbrignull]

The python-dep tests didn't appear to be running on the cases that were intended. I don't think the use of include was correct, or possibly there might be a bug in how it's being parsed / handled. Either way I'm pretty sure it's not how that feature was intended to be used. I'm going off off the information at https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#example-including-additional-values-into-combinations

This PR removes the use of include in favour of a more standard matrix, though this did require splitting one job up into two.

If this works then it means we'll now be running 26 jobs. So another question is is it necessary to run this many variations?

Merge / deployment checklist

• Confirm this change is backwards compatible with existing workflows.
• Confirm the readme has been updated if necessary.

[aeisenberg]

26 jobs does seem large, but they are quite fast. Two suggestions (not sure if they will work):

• add exclude clauses so that we trim the number of jobs down a bit

• or maybe keep the original matrix and add this (not sure if this works, but maybe worth a try?):

  matrix:
  os: [ubuntu-latest, macos-latest]test_dir: []test_script: []include:
  - test_dir: python-setup/tests/pipenv/requests-2test_script: $GITHUB_WORKSPACE/python-setup/tests/check_requests_123.sh 2
  - test_dir: python-setup/tests/pipenv/requests-3test_script: $GITHUB_WORKSPACE/python-setup/tests/check_requests_123.sh 3...etc...

[adityasharad]

The includes in the original matrix weren't working because they didn't mention os, so they weren't getting added to any existing configurations. I think this looks reasonable, will do a more careful review shortly.

[adityasharad]

Looks good. The number of matrixed jobs is ok, since these aren't in the same workflow as the other PR checks. Some minor suggestions for factoring out and quoting matrix values via environment variables.

[robertbrignull]

I've moved the uses of the matrix vars to environment variables for safety like you suggested. Seems to be working.

[robertbrignull]

Seems to be working.

Knew I shouldn't have posted that. I forgot the windows version runs with powershell. Hopefully that's got it now.

[adityasharad]

PowerShell env vars get me every time!
This looks good, I think I need to adjust the branch protection rules I added last week since the Windows check names have changed.

[robertbrignull]

I think I need to adjust the branch protection rules I added last week since the Windows check names have changed.

Ugh, that's annoying.

So it's changed from test-setup-python-scripts-windows (python-setup/tests/pipenv/requests-2, 2) to test-setup-python-scripts-windows (pipenv, 2).

So if the rules were updated that would allow this to pass but would then cause problems for all other open PRs. Probably we'll need to just update the rules and merge this PR immediately and deal with any fallout. Or temporarily remove the branch protection rules for these checks are reintroduce them after a period.

[adityasharad]

I've removed those checks from the required list for now, and will reenable them once we merge this PR. At that point all open PRs will need to update from main anyway, so there shouldn't be much fallout from enabling the new checks.