Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork938
Configure tox#1667
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Merged
Configure tox#1667
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
It is not completely working yet.
This fixes a problem where the tests for fetching a nonexistent refprompt like "Enter passphrase for key '/home/USERNAME/.ssh/id_rsa':"and block, if the repository on the machine where the tests arebeing run has the remote set up using an SSH URL.This passes through all environment variables whose names startwith SSH_, even though it should be enough to pass SSH_AGENT_PIDand SSH_AUTH_SOCK through, at least for this particular issue.
Other environments would still be run even after mypy has failed,but to avoid having tox runs be unnecessarily inconsistent with themypy step in the pythonpackage.yml CI workflow, and also becauseGitPython is not currently expected to pass mypy checks, this keepsmypy errors from causing the whole tox run to be reported asfailed.
The main use of this, similar to the step at the end ofpythonpackage.yml, is to find errors produced by building.However, actual documentation *is* built, and unlike other toxenvironments, running this one actually writes outside the .tox/directory, creating the documentation in the usual targetlocation. For that reason, this environment is omitted from theenv_list, so that it does not run by default and unexpectedlyoverwrite documentation that may recently have been built beforechanges are made that could cause generated documentation to bedifferent.
Byron approved these changesSep 19, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Thanks a million for all of your great work!
I remember having hadtox.ini for a while, but don't recall why it was removed.
Furthermore, I follow the reasoning behind this PR and agree that at this point it's probably not worth mentioning it directly in the README. The simplest way to run tests, albeit only in the current interpreter, still seems to be to usevenv. I also think it's valid to let CI test everything. But what happens if a test fails in a python version that one doesn't have locally? Then havingtox would certainly help fixing it.
Maybe in follow-up PR the contribution guide could be adjusted to includetox related development workflows, along with hints about why one would use it. That way it's presented as more of an evolution rather than a requirement. Also, I haven't heard of an alternative sotox seems to have uniquely useful capabilities, whether one likes its output style or not.
Regardingtox on CI, I'd definitely prefer keeping CI as is. Usingtox related workflows certainly has advantages when starting a project as it saves dealing with GHA oneself, but since we have that already, usingtox-specific workflow seems to abstract and occlude. Or in other words, right now I'd feel more in control with GHA, it's the devil I know.
Uh oh!
There was an error while loading.Please reload this page.
There is also |
Right! Since |
renovatebot referenced this pull request in allenporter/flux-localSep 23, 2023
[](https://renovatebot.com)This PR contains the following updates:| Package | Change | Age | Adoption | Passing | Confidence ||---|---|---|---|---|---|| [GitPython](https://togithub.com/gitpython-developers/GitPython) |`==3.1.36` -> `==3.1.37` |[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|---### Release Notes<details><summary>gitpython-developers/GitPython (GitPython)</summary>###[`v3.1.37`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.37):- a proper fixCVE-2023-41040[CompareSource](https://togithub.com/gitpython-developers/GitPython/compare/3.1.36...3.1.37)#### What's Changed- Improve Python version and OS compatibility, fixing deprecations by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1654](https://togithub.com/gitpython-developers/GitPython/pull/1654)- Better document env_case test/fixture and cwd by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1657](https://togithub.com/gitpython-developers/GitPython/pull/1657)- Remove spurious executable permissions by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1658](https://togithub.com/gitpython-developers/GitPython/pull/1658)- Fix up checks in Makefile and make them portable by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1661](https://togithub.com/gitpython-developers/GitPython/pull/1661)- Fix URLs that were redirecting to another license by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1662](https://togithub.com/gitpython-developers/GitPython/pull/1662)- Assorted small fixes/improvements to root dir docs by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1663](https://togithub.com/gitpython-developers/GitPython/pull/1663)- Use venv instead of virtualenv in test_installation by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1664](https://togithub.com/gitpython-developers/GitPython/pull/1664)- Omit py_modules in setup by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1665](https://togithub.com/gitpython-developers/GitPython/pull/1665)- Don't track code coverage temporary files by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1666](https://togithub.com/gitpython-developers/GitPython/pull/1666)- Configure tox by [@​EliahKagan](https://togithub.com/EliahKagan)in[https://github.com/gitpython-developers/GitPython/pull/1667](https://togithub.com/gitpython-developers/GitPython/pull/1667)- Format tests with black and auto-exclude untracked paths by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1668](https://togithub.com/gitpython-developers/GitPython/pull/1668)- Upgrade and broaden flake8, fixing style problems and bugs by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1673](https://togithub.com/gitpython-developers/GitPython/pull/1673)- Fix rollback bug in SymbolicReference.set_reference by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1675](https://togithub.com/gitpython-developers/GitPython/pull/1675)- Remove `@NoEffect` annotations by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1677](https://togithub.com/gitpython-developers/GitPython/pull/1677)- Add more checks for the validity of refnames by[@​facutuesca](https://togithub.com/facutuesca) in[https://github.com/gitpython-developers/GitPython/pull/1672](https://togithub.com/gitpython-developers/GitPython/pull/1672)**Full Changelog**:gitpython-developers/GitPython@3.1.36...3.1.37</details>---### Configuration📅 **Schedule**: Branch creation - At any time (no schedule defined),Automerge - At any time (no schedule defined).🚦 **Automerge**: Enabled.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick therebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about this updateagain.---- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, checkthis box---This PR has been generated by [MendRenovate](https://www.mend.io/free-developer-tools/renovate/). Viewrepository job log[here](https://developer.mend.io/github/allenporter/flux-local).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi45Ny4xIiwidXBkYXRlZEluVmVyIjoiMzYuOTcuMSIsInRhcmdldEJyYW5jaCI6Im1haW4ifQ==-->Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
%5d(https%3a%2f%2frenovatebot.com){kind=link}
I think |
To my mind, the less python is needed, the better, so I am happy to stay with the little Makefile used here 😅. |
otc-zuulbot pushed a commit to opentelekomcloud-infra/eyes_on_docs that referenced this pull requestOct 25, 2023
Bump gitpython from 3.1.35 to 3.1.37Bumps gitpython from 3.1.35 to 3.1.37.Release notesSourced from gitpython's releases.3.1.37 - a proper fixCVE-2023-41040What's ChangedImprove Python version and OS compatibility, fixing deprecations by @EliahKagan ingitpython-developers/GitPython#1654Better document env_case test/fixture and cwd by @EliahKagan ingitpython-developers/GitPython#1657Remove spurious executable permissions by @EliahKagan ingitpython-developers/GitPython#1658Fix up checks in Makefile and make them portable by @EliahKagan ingitpython-developers/GitPython#1661Fix URLs that were redirecting to another license by @EliahKagan ingitpython-developers/GitPython#1662Assorted small fixes/improvements to root dir docs by @EliahKagan ingitpython-developers/GitPython#1663Use venv instead of virtualenv in test_installation by @EliahKagan ingitpython-developers/GitPython#1664Omit py_modules in setup by @EliahKagan ingitpython-developers/GitPython#1665Don't track code coverage temporary files by @EliahKagan ingitpython-developers/GitPython#1666Configure tox by @EliahKagan ingitpython-developers/GitPython#1667Format tests with black and auto-exclude untracked paths by @EliahKagan ingitpython-developers/GitPython#1668Upgrade and broaden flake8, fixing style problems and bugs by @EliahKagan ingitpython-developers/GitPython#1673Fix rollback bug in SymbolicReference.set_reference by @EliahKagan ingitpython-developers/GitPython#1675Remove@NoEffect annotations by @EliahKagan ingitpython-developers/GitPython#1677Add more checks for the validity of refnames by @facutuesca ingitpython-developers/GitPython#1672Full Changelog: gitpython-developers/GitPython@3.1.36...3.1.37Commitsb27a89f fix makefile to compare commit hashes only0bd2890 prepare next release832b6ee remove unnecessary list comprehension to fix CIe98f57b Merge pull request #1672 from trail-of-forks/robust-refname-checks1774f1e Merge pull request #1677 from EliahKagan/no-noeffecta4701a0 Remove@NoEffect annotationsd40320b Merge pull request #1675 from EliahKagan/rollbackd1c1f31 Merge pull request #1673 from EliahKagan/flake8e480985 Tweak rollback logic in log.to_fileff84b26 Refactor try-finally cleanup in git/Additional commits viewable in compare viewDependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting@dependabot rebase.Dependabot commands and optionsYou can trigger Dependabot actions by commenting on this PR:@dependabot rebase will rebase this PR@dependabot recreate will recreate this PR, overwriting any edits that have been made to it@dependabot merge will merge this PR after your CI passes on it@dependabot squash and merge will squash and merge this PR after your CI passes on it@dependabot cancel merge will cancel a previously requested merge and block automerging@dependabot reopen will reopen this PR if it is closed@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)You can disable automated security fix PRs for this repo from the Security Alerts page.Reviewed-by: Vladimir Vshivkov
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 12, 2024
This is to make it so simple `tox` usage has the expected propertyof leaving all source code files in the working tree unchanged.As noted inc66257e, linting how sometimes performs auto-fixessincegitpython-developers#1862, and the pre-commit command in tox.ini, which had alsorun `black --check`, will do even more file editing withgitpython-developers#1865.The bifurcation for black into separate mutating and non-mutatinghooks, introduced in5d8ddd9 (gitpython-developers#1693), is not carried over into Ruffautoformatting ingitpython-developers#1865. But also it:- Was not necessarily a good approach, and likely should not be preserved in any form. It is an unusual and unintuitive use of pre-commit. (It can be brought back if no better approach is found, though.)- Was done to avoid a situation where it was nontrivial to set up necessary dependencies for linting in the GitPython virtual environment itself, because flake8 and its various plugins would have to be installed. They were not listed in any existing or newly introduced extra (for example, they were not added to test-requirements.txt) in part in the hope that they would all be replaced by Ruff, which happened ingitpython-developers#1862.- Already does not achieve its goal sincegitpython-developers#1862, since it was (probably rightly) not extended to Ruff linting to use/omit --fix.Now that Ruff is being used, people can run `pip install ruff` in avirtual environment, then run the `ruff` command however they like.This takes the place of multiple tools and plugins.This commit *avoids* doing any of the following, even though it maybe useful to do them later:- This does not give specific instructions in the readme for installing and running ruff (andc66257e before this also omits that). This can be added later and the best way to document it may depend on some other upcoming decisions (see below).- This does not add ruff to the test extra or as any other kind of extra or optional dependency. Although the test extra currently contains some packages not used for running unit tests, such as pre-commit and mypy, adding Ruff will cause installation to take a long time and/or or fail on some platforms like Cygwin where Ruff has to be built from (Rust) source code. This can be solved properly by reorganizing the extras, but that is likely to wait until they are expressed completely inside pyproject.toml rather than one per requirements file (see discussion in comments ingitpython-developers#1716 for general information about possible forthcoming changes in how the project is defined).- This does not update tox.ini to run ruff directly, which could be done by splitting the current lint tox environment into two or three environments for Python linting, Python autoformatting, and the miscellaneous other tasks performed by pre-commit hooks, only the latter of which would use the pre-commit command. Whether and how this should be done may depend on other forthcoming changes.- This does not remove or update the Makefile "lint" target. That target should perhaps not have been added, and was always meant to be improved/replaced, since everything else in the top-level Makefile is for building and publishing releases. See5d15063 (gitpython-developers#1693). This is likewise not done now since it may depend on as-yet unmerged changes and tooling decisions not yet made. It should be feasible to do together when further updating tox.ini.- This does not update tox.ini, Makefile, or the lint.yml GitHub Actions workflow to omit the manual hook-stage, which will be unused as ofgitpython-developers#1865. This would complicate integration of changes, and once it is known that it won't be needed, it can removed.The situation with the tox "lint" environment is thus now similarto that of the tox "html" environment when it was added ine6ec6c8(gitpython-developers#1667), until it was improved inf094909 (gitpython-developers#1693) to run withproper isolation.
EliahKagan added a commit to EliahKagan/GitPython that referenced this pull requestMar 13, 2024
This is to make it so simple `tox` usage has the expected propertyof leaving all source code files in the working tree unchanged.Linting how sometimes performs auto-fixes sincegitpython-developers#1862, and thepre-commit command in tox.ini, which had also run `black --check`,will do even more file editing due to the changes ingitpython-developers#1865.The bifurcation for black into separate mutating and non-mutatinghooks, introduced in5d8ddd9 (gitpython-developers#1693), was not carried over intoRuff autoformatting ingitpython-developers#1865. But also it:- Was not necessarily a good approach, and likely should not be preserved in any form. It was an unusual and unintuitive use of pre-commit. (It can be brought back if no better approach is found, though.)- Was done to avoid a situation where it was nontrivial to set up necessary dependencies for linting in the GitPython virtual environment itself, because flake8 and its various plugins would have to be installed. They were not listed in any existing or newly introduced extra (for example, they were not added to test-requirements.txt) in part in the hope that they would all be replaced by Ruff, which happened ingitpython-developers#1862.- Already did not achieve its goal as ofgitpython-developers#1862, since it was (probably rightly) not extended to Ruff linting to use/omit --fix.Now that Ruff is being used, people can run `pip install ruff` in avirtual environment, then run the `ruff` command however they like.This takes the place of multiple tools and plugins.The situation with the tox "lint" environment is thus now similarto that of the tox "html" environment when it was added ine6ec6c8(gitpython-developers#1667), until it was improved inf094909 (gitpython-developers#1693) to run withproper isolation.
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
None yet
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
This adds a
tox.inifile so thattoxcan be run locally to test on 3.7, 3.8, 3.9, 3.10, 3.11, and 3.12, or whichever of them are available, and also run the lint, formatting, and style checks.Because CI also tests building the documentation, and it is convenient to be able to use
toxto check all the things CI checks (albeit on just one's local platform), I have added a tox environment for that too. But because it actually writes to the build directory, I have not listed it inenv_listat the top, so it doesn't run automatically--it can still be run withtox -e html. That could be improved in the future, but I think reasonable ways of doing so would involve modifyingdoc/Makefileto make it more configurable, and that seems outside the proper scope of this PR.This PR doesnot use
toxon CI. It is possible to usetoxto drive tests on CI, such as with thetox-gh-actionsplugin, but this does not do that, because:toxfor this project is not primarily for what it brings to CI nor even for making local and CI based testing more consistent.toxworkflow will sometimes make results harder to read, because they areno longer divided into top-level steps belonging to the test job and clearly shown as separate. This doesn't mean it shouldn't be done, and either manual customization or alternatives totox-gh-actionssuch astox-gh-matrixmight help.toxon CI are for when thetoxenvironments are more granular than the CI jobs. This happens in projects that need to parameterize their test runs not just on OS and Python versionbut also on dependency versions (while avoiding the proliferation of separate CI jobs), which I don't think is applicable here.So while I suspect it may eventually be a good idea to use
toxon CI, I don't think it should be done until it is clearlyno worse than the current approach and other readily available approaches.My intention is also not for
toxto replace other ways of running tests when developing locally. During routine development,toxmay be too slow or its output too complicated, for regularly checking the status of some or all tests. Furthermore,pre-commitseems to be working great for the lint checks--and the lint tox environment I configured uses it, to as to pick up any new or changed linting tools automatically. (Also, not everyone likestox.)For these reasons, I have not modified the readme to replace anything with
tox-based instructions. However, the readme could be modified toalso talk abouttox. I am not sure if that should be done either, because the users most interested in usingtoxare those already familiar with it, and they will notice thetox.ini. Furthermore, I think it may be best to see how it works out (for example, should it really have the environment for testing that we can build docs by actually building them?) before adding it to the readme or other project documentation. However, if you'd liketoxinstructions, or a mention of it, etc., added the readme, I'd be pleased to add it (as well as to make other requested changes).Some other considerations:
toxto run tests was being prompted to enter my passphrase for some tests. Because that completely breaks the useful automationtoxprovides, I allowedtoxto pass environment variables whose names start withSSH_through to its test environments. It would probably be sufficient to pass throughSSH_AGENT_PIDandSSH_AUTH_SOCK; I'd be pleased to change it to just that, if requested. (If other forms of authentication than SSH that I'm not using rely on environment variables being set, then that might still be broken.)toxenvironments for everything other than the tests are specified as 3.9, to support Cygwin and to allow results to be compared between Cygwin and other platforms. Also, because Cygwin doesn't have 3.10, 3.11, or 3.12, I have not specifiedskip_missing_interpreters = true, leaving it to the default offalse. Nonetheless, I do not claimtoxbehaves perfectly on Cygwin in this configuration. I sometimes have problems where it tries to use Python interpreters from my native system before eventually finding something it can really use.toxfromrequirements-dev.txt, and not added it totest-requirements.txtsince it should rarely be installed together with other project dependencies in the same environment. It's possible to installtoxin a virtual environment that one is using for the project, but this is not typically done, becausetoxcreates its own environments. It's best to installtoxwithpipxwhen possible. Furthermore, if any version oftoxis available, and that version is lower than the version specified intox.ini, thentoxwill bootstrap a virtual environment and install a newer version of itself in it. Of course, one can still runpip install toxin a virtual environment (or the global environment) if one wishes to gettoxthat way.toxconfigurationinpyproject.tomlto avoid adding yet another top-level file. But I decided against it. It forgoes the discoverability benefits noted above in my reasoning about why I haven't mentionedtoxin the readme. Also,it has to be given as one giant string, and editors (and GitHub) don't currently special-case that with useful syntax highlighting.