Skip to content

Commit

Permalink
Create openssl-version, allowing for reporting of openssl-version wit…
Browse files Browse the repository at this point in the history 
…hout requiring tox-run-command (and the trouble that adds).
  • Loading branch information
@jaraco
jaraco committed Oct 10, 2019
1 parent 7d0dcdb commit f791b13
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
6 changes: 2 additions & 4 deletions .github/workflows/python-tests.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,8 @@ jobs:
+ repr(ssl.OPENSSL_VERSION_NUMBER))"
env: ${{ matrix.env }}
- name: Log PyOpenSSL version
run: >-
python -m tox --run-command
"{envpython} -m OpenSSL.debug"
|| :
run: |
python -m tox -e openssl-version
env: ${{ matrix.env }}
- name: Test with tox
run: |
Expand Down
6 changes: 4 additions & 2 deletions tox.ini
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
[tox]
envlist = python
minversion = 3.13.2
requires =
tox-run-command >= 0.4

[testenv]
deps =
Expand Down Expand Up @@ -32,6 +30,10 @@ setenv =
PYTHONDONTWRITEBYTECODE=x
WEBTEST_INTERACTIVE=false

[testenv:openssl-version]
commands =
python -m OpenSSL.debug

[testenv:build-docs]
basepython = python3.7
description = Build The Docs
Expand Down

9 comments on commit f791b13

@webknjaz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jaraco with this implementation it's not guaranteed that it'll report the same info since the man test env because you create a new virtualenv in a separate step which may even hit a different interpreter.

@jaraco
Copy link
Member Author

@jaraco jaraco commented on f791b13 Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I see it, because testenv:openssl-version inherits the settings from testenv, only overriding commands and because in the steps, both environments are created with python -m tox and because envlist = python (meaning use the same version of Python as tox), those envs, although they're not the same envs, should have identical makeup, at least for a Github workflows run. If someone is running these commands locally and running the commands at different times, I could see how the dependencies could resolve differently, but that seems like a low risk.

I do regret not having linked the issue in the commit. I can't even find that I registered an issue somewhere, probably because I was already layers deep on an issue I hadn't intended to be troubleshooting.

It occurs to me, it should be possible to avoid creating a separate environment altogether and simply invoke .tox/python/bin/python -m OpenSSL.debug.

@jaraco
Copy link
Member Author

@jaraco jaraco commented on f791b13 Oct 20, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be possible to avoid creating a separate environment altogether

That's what I did in bfb8e87 and 33fa3f5.

@jaraco
Copy link
Member Author

@jaraco jaraco commented on f791b13 Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. That technique may not work because Github workflows run on Windows also, which doesn't have .tox/python/bin (has .tox/python/Scripts instead).

@jaraco
Copy link
Member Author

@jaraco jaraco commented on f791b13 Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 94c7e09, I've restored tox-run-command, but limited its scope to just the Github workflows.

@webknjaz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, you mentioned earlier that we have to avoid OS-dependent things. That's the reason I introduced this plugin. Besides, Travis CI is also capable of running Windows so we may need to reconsider the hardcoded command there as well.

@webknjaz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could see how the dependencies could resolve differently, but that seems like a low risk.

Actually, I was thinking along the lines of a scenario when somebody adds extra steps into the CI config between running those commands in the future. This future person may not be attentive enough and wouldn't realize the consequences.

@jaraco
Copy link
Member Author

@jaraco jaraco commented on f791b13 Oct 20, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides, Travis CI is also capable of running Windows so we may need to reconsider the hardcoded command there as well.

In that case, I'd lean toward injecting tox-run-command separately in each environment. Adding this dependency to tox.requires causes new virtualenvs to be created just to run tox, adding complexity to a workflow and potentially leading to other problems to debug (such as I encountered in tox-dev/tox#1435). At some point, we may want to make tox-run-command or other tox plugins available in the tox.requires, but I'd like to defer that for as long as possible, in particular when a developer's workflow demands that tooling (and not just the CI environments).

@webknjaz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I imagine that with the introduction of --devenv would be unnecessary.

Please sign in to comment.