-
-
Notifications
You must be signed in to change notification settings - Fork 904
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue and test deprecation warnings #1886
Issue and test deprecation warnings #1886
Commits on Mar 28, 2024
-
Test that deprecated Diff.renamed property warns
This starts on a test.deprecation subpackage for deprecation tests. Having these tests in a separate directory inside the test suite may or may not be how they will ultimately be orgnaized, but it has two advantages: - Once all the tests are written, it should be easy to see what in GitPython is deprecated. - Some deprecation warnings -- those on module or class attribute access -- will require the introduction of new dynamic behavior, and thus run the risk of breaking static type checking. So that should be checked for, where applicable. But currently the test suite has no type annotations and is not checked by mypy. Having deprecation-related tests under the same path will make it easier to enable mypy for just this part of the test suite (for now). It is also for this latter reason that the one test so far is written without using the GitPython test suite's existing fixtures whose uses are harder to annotate. This may be changed if warranted, though some of the more complex deprecation-related tests may benefit from being written as pure pytest tests. Although a number of deprecated features in GitPython do already issue warnings, Diff.renamed is one of the features that does not yet do so. So the newly introduced test will fail until that is fixed in the next commit.
Configuration menu - View commit details
-
Copy full SHA for 2382891 - Browse repository at this point
Copy the full SHA 2382891View commit details -
Configuration menu - View commit details
-
Copy full SHA for e7dec7d - Browse repository at this point
Copy the full SHA e7dec7dView commit details -
Fix exception in Popen.__del__ in test on Windows
The newly introduced test would pass, but show: Exception ignored in: <function Popen.__del__ at 0x000002483DE14900> Traceback (most recent call last): File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.12_3.12.752.0_x64__qbz5n2kfra8p0\Lib\subprocess.py", line 1130, in __del__ self._internal_poll(_deadstate=_maxsize) File "C:\Program Files\WindowsApps\PythonSoftwareFoundation.Python.3.12_3.12.752.0_x64__qbz5n2kfra8p0\Lib\subprocess.py", line 1575, in _internal_poll if _WaitForSingleObject(self._handle, 0) == _WAIT_OBJECT_0: ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ OSError: [WinError 6] The handle is invalid This was due to how, at least for now, the deprecation warning tests are not using GitPython's normal fixtures, which help clean things up on Windows. This adds a small amount of ad-hoc cleanup. It also moves the acquisition of the Diff object into a pytest fixture, which can be reused for the immediately forthcoming test that the preferred property does not warn.
Configuration menu - View commit details
-
Copy full SHA for a8f109c - Browse repository at this point
Copy the full SHA a8f109cView commit details -
Configuration menu - View commit details
-
Copy full SHA for fffa6ce - Browse repository at this point
Copy the full SHA fffa6ceView commit details -
Configuration menu - View commit details
-
Copy full SHA for bc111b7 - Browse repository at this point
Copy the full SHA bc111b7View commit details -
Decompose new fixture logic better
This will be even more helpful when testing a deprecated member of the Commit class (not yet done).
Configuration menu - View commit details
-
Copy full SHA for e3728c3 - Browse repository at this point
Copy the full SHA e3728c3View commit details -
Extract no-deprecation-warning asserter as a context manager
+ Remove the TODO suggesting the diff not be computed from a real commit, since we're going to have the logic for that in use in forthcoming commit-specific deprecation warning tests anyway.
Configuration menu - View commit details
-
Copy full SHA for ff4b58d - Browse repository at this point
Copy the full SHA ff4b58dView commit details -
Test that the deprecated Commit.trailers property warns
And that the non-deprecated recommended alternative trailers_list and trailers_dict properties do not warn. The test that trailers warns does not yet pass yet, because it has not yet been made to warn.
Configuration menu - View commit details
-
Copy full SHA for 2c52696 - Browse repository at this point
Copy the full SHA 2c52696View commit details -
Configuration menu - View commit details
-
Copy full SHA for 03464d9 - Browse repository at this point
Copy the full SHA 03464d9View commit details -
Configuration menu - View commit details
-
Copy full SHA for 9d096e0 - Browse repository at this point
Copy the full SHA 9d096e0View commit details -
Use the :exc: Sphinx role for DeprecationWarning
Python warnings are exceptions, to facilitate converting them to errors by raising them. Thus the more specific :exc: role applies and is a better fit than the more general :class: role.
Configuration menu - View commit details
-
Copy full SHA for 21c2b72 - Browse repository at this point
Copy the full SHA 21c2b72View commit details -
Test that subclassing deprecated git.util.Iterable warns
And that subclassing the strongly preferred git.util.Iterable class does not warn.
Configuration menu - View commit details
-
Copy full SHA for ca385a5 - Browse repository at this point
Copy the full SHA ca385a5View commit details -
Call repo.close() instead of manually collecting
The code this replaces in the `commit` pytest fixture seems not to be needed anymore, and could arguably be removed now with no further related changes. But whether it is needed depends on subtle and sometimes nondeterministic factors, and may also vary across Python versions. To be safe, this replaces it with a call to the Repo instance's close method, which is in effect more robust than what was being done before, as it calls clear_cache on the the Git object that the Repo object uses, does a gitdb/smmap collection, and on Windows calls gc.collect both before and after that collection. This may happen immediately anyway if the Repo object is not reachable from any cycle, since the reference count should go to zero after each of the deprecation warning tests (the fixture's lifetime is that of the test case), and Repo.close is called in Repo.__del__. But this makes it happen immediately even if there is a cycle.
Configuration menu - View commit details
-
Copy full SHA for 8bbcb26 - Browse repository at this point
Copy the full SHA 8bbcb26View commit details -
Better name and document the basic deprecation test module
The other deprecation test modules this refers to don't exist yet but will be introduced soon.
Configuration menu - View commit details
-
Copy full SHA for b8ce990 - Browse repository at this point
Copy the full SHA b8ce990View commit details
Commits on Mar 29, 2024
-
Annotate basic deprecation tests; have mypy scan it
- Add type hints to test/deprecation/basic.py. As its module docstring (already) notes, that test module does not contain code where it is specifically important that it be type checked to verify important properties of the code under test. However, other test.deprecation.* modules will, and it is much more convenient to be able to scan the whole directory than the directory except for one file. (Less importantly, for the deprecation tests to be easily readable as a coherent whole, it makes sense that all, not just most, would have annotations.) - Configure mypy in pyproject.toml so it can be run without arguments (mypy errors when run that way unless configured), where the effect is to scan the git/ directory, as was commonly done before, as well as the test/deprecation/ directory. - Change the CI test workflow, as well as tox.ini, to run mypy with no arguments instead of passing `-p git`, so that it will scan both the git package as it had before and the test.deprecation package (due to the new pyproject.toml configuration). - Change the readme to recommend running it that way, too.
Configuration menu - View commit details
-
Copy full SHA for 61273aa - Browse repository at this point
Copy the full SHA 61273aaView commit details -
Configuration menu - View commit details
-
Copy full SHA for b7a3d8c - Browse repository at this point
Copy the full SHA b7a3d8cView commit details -
Test attribute access and importing separately
Rather than only testing attribute access.
Configuration menu - View commit details
-
Copy full SHA for 105f500 - Browse repository at this point
Copy the full SHA 105f500View commit details -
Configuration menu - View commit details
-
Copy full SHA for 859e38c - Browse repository at this point
Copy the full SHA 859e38cView commit details -
Hoist
import git
to module level in test moduleBecause it's going to be necessary to express things in terms of it in parametrization markings, in order for mypy to show the expected errors for names that are available dynamically but deliberately static type errors.
Configuration menu - View commit details
-
Copy full SHA for 46a739d - Browse repository at this point
Copy the full SHA 46a739dView commit details -
Test static typing of private module aliases
This tests that mypy considers them not to be present. That mypy is configured with `warn_unused_ignores = true` is key, since that is what verifies that the type errors really do occur, based on the suppressions written for them.
Configuration menu - View commit details
-
Copy full SHA for a2df3a8 - Browse repository at this point
Copy the full SHA a2df3a8View commit details -
Configuration menu - View commit details
-
Copy full SHA for a15a830 - Browse repository at this point
Copy the full SHA a15a830View commit details -
Configuration menu - View commit details
-
Copy full SHA for dbaa535 - Browse repository at this point
Copy the full SHA dbaa535View commit details -
Configuration menu - View commit details
-
Copy full SHA for d00c843 - Browse repository at this point
Copy the full SHA d00c843View commit details -
Configuration menu - View commit details
-
Copy full SHA for 983fda7 - Browse repository at this point
Copy the full SHA 983fda7View commit details -
Configuration menu - View commit details
-
Copy full SHA for 19acd4c - Browse repository at this point
Copy the full SHA 19acd4cView commit details -
Configuration menu - View commit details
-
Copy full SHA for f39bbb5 - Browse repository at this point
Copy the full SHA f39bbb5View commit details -
Configuration menu - View commit details
-
Copy full SHA for aee7078 - Browse repository at this point
Copy the full SHA aee7078View commit details -
Fix brittle way of checking warning messages
Which was causing a type error.
Configuration menu - View commit details
-
Copy full SHA for 7f4a191 - Browse repository at this point
Copy the full SHA 7f4a191View commit details -
Configuration menu - View commit details
-
Copy full SHA for d08a576 - Browse repository at this point
Copy the full SHA d08a576View commit details -
Configuration menu - View commit details
-
Copy full SHA for 9d58e6d - Browse repository at this point
Copy the full SHA 9d58e6dView commit details -
Finish reorganizing; fix assertion for duplicated messages
To support the changes, this adds typing-extensions as a test dependency, since we are now importing from it in a test module. But this should probably be required and used conditionally based on whether the Python version has assert_type in its typing module.
Configuration menu - View commit details
-
Copy full SHA for 45c128b - Browse repository at this point
Copy the full SHA 45c128bView commit details -
Add imports so pyright recognizes refs and index
pyright still reports git.util as private, as it should. (mypy does not, or does not by default, report private member access. GitPython does not generally use pyright as part of development at this time, but I am checking some code with it during the process of writing the new tests.)
Configuration menu - View commit details
-
Copy full SHA for 247dc15 - Browse repository at this point
Copy the full SHA 247dc15View commit details -
Expand and clarify test module docstring
About why there are so many separate mypy suppressions even when they could be consolidated into a smaller number in some places.
Configuration menu - View commit details
-
Copy full SHA for b05963c - Browse repository at this point
Copy the full SHA b05963cView commit details -
Configuration menu - View commit details
-
Copy full SHA for 074bbc7 - Browse repository at this point
Copy the full SHA 074bbc7View commit details -
Pick a better name for _MODULE_ALIAS_TARGETS
And add a docstring to document it, mainly to clarify that util is intentionally omitted from that constant.
Configuration menu - View commit details
-
Copy full SHA for 18608e4 - Browse repository at this point
Copy the full SHA 18608e4View commit details -
Use typing_extensions only if needed
This makes the typing-extensions test dependency < 3.11 only, and conditionally imports assert_type from typing or typing_extensions depending on the Python version.
Configuration menu - View commit details
-
Copy full SHA for 1f290f1 - Browse repository at this point
Copy the full SHA 1f290f1View commit details -
This omits strict=True, which is only supported in Python 3.10 and later, and instead explicitly asserts that the arguments are the same length (which is arguably better for its explicitness anyway).
Configuration menu - View commit details
-
Copy full SHA for 7a4f7eb - Browse repository at this point
Copy the full SHA 7a4f7ebView commit details -
Configuration menu - View commit details
-
Copy full SHA for 5977a6e - Browse repository at this point
Copy the full SHA 5977a6eView commit details -
Configuration menu - View commit details
-
Copy full SHA for 5b1fa58 - Browse repository at this point
Copy the full SHA 5b1fa58View commit details -
And rename test_attributes to test_toplevel accordingly.
Configuration menu - View commit details
-
Copy full SHA for a07be0e - Browse repository at this point
Copy the full SHA a07be0eView commit details -
Configuration menu - View commit details
-
Copy full SHA for d4917d0 - Browse repository at this point
Copy the full SHA d4917d0View commit details -
Configuration menu - View commit details
-
Copy full SHA for f4e5f42 - Browse repository at this point
Copy the full SHA f4e5f42View commit details -
Configuration menu - View commit details
-
Copy full SHA for d54f851 - Browse repository at this point
Copy the full SHA d54f851View commit details -
Configuration menu - View commit details
-
Copy full SHA for aaf046a - Browse repository at this point
Copy the full SHA aaf046aView commit details -
Configuration menu - View commit details
-
Copy full SHA for 84d734d - Browse repository at this point
Copy the full SHA 84d734dView commit details -
Configuration menu - View commit details
-
Copy full SHA for 3a621b3 - Browse repository at this point
Copy the full SHA 3a621b3View commit details -
Configuration menu - View commit details
-
Copy full SHA for 05e0878 - Browse repository at this point
Copy the full SHA 05e0878View commit details -
Configuration menu - View commit details
-
Copy full SHA for 3fe2f15 - Browse repository at this point
Copy the full SHA 3fe2f15View commit details -
Use names directly on other tests
The tests are written broadly (per the style elsewhere in this test suite), but narrowing the message-checking tests in this specific way has the further advantage that the logic of the code under test will be less reflected in the logic of the tests, so that bugs are less likely to be missed by being duplicated across code and tests.
Configuration menu - View commit details
-
Copy full SHA for 246cc17 - Browse repository at this point
Copy the full SHA 246cc17View commit details -
Configuration menu - View commit details
-
Copy full SHA for d7b6b31 - Browse repository at this point
Copy the full SHA d7b6b31View commit details -
Configuration menu - View commit details
-
Copy full SHA for 96089c8 - Browse repository at this point
Copy the full SHA 96089c8View commit details -
Configuration menu - View commit details
-
Copy full SHA for a0ef537 - Browse repository at this point
Copy the full SHA a0ef537View commit details -
Configuration menu - View commit details
-
Copy full SHA for 52e7360 - Browse repository at this point
Copy the full SHA 52e7360View commit details -
Configuration menu - View commit details
-
Copy full SHA for e3675a0 - Browse repository at this point
Copy the full SHA e3675a0View commit details -
Configuration menu - View commit details
-
Copy full SHA for 4857ff0 - Browse repository at this point
Copy the full SHA 4857ff0View commit details -
Configuration menu - View commit details
-
Copy full SHA for 488cc13 - Browse repository at this point
Copy the full SHA 488cc13View commit details -
Configuration menu - View commit details
-
Copy full SHA for 19b3c08 - Browse repository at this point
Copy the full SHA 19b3c08View commit details -
Configuration menu - View commit details
-
Copy full SHA for 28bd4a3 - Browse repository at this point
Copy the full SHA 28bd4a3View commit details -
Refine deprecated module attributes and their warnings
- In the top-level git module, import util from git.index so there is no ambiguity for tools in detecting which module the util attribute of the top-level git module (i.e., git.util in an *expression*) is, even though it's subsequently deleted (and then dynamically supplied when requested, in a way that is opaque to static type checkers due to being only when `not TYPE_CHECKING`). This seems to be necessary for some tools. Curiously, guarding the del statement under `not TYPE_CHECKING` seems *not* to be needed by any tools of any kind. It should still possibly be done, but that's not included in these changes. - Add the missing deprecation warning for git.types.Lit_commit_ish. - When importing the warnings module, do so with a top-level import as in the other GitPython modules that have long (and reasonably) done so. In git/__init__.py, there already had to be an import of importlib, which seemed like it should be done locally in case of delays. Neither the importlib module nor any of its submodules were already imported anywhere in GitPython, and the code that uses it will most often not be exercised. So there is a potential benefit to avoiding loading it when not needed. When writing a local import for that, I had included the warnings module as a local import as well. But this obscures the potential benefit of locally importing importlib, and could lead to ill-advised changes in the future based on the idea that the degree of justification to be local imports was the same for them both.
Configuration menu - View commit details
-
Copy full SHA for dffa930 - Browse repository at this point
Copy the full SHA dffa930View commit details -
Configuration menu - View commit details
-
Copy full SHA for 7ab27c5 - Browse repository at this point
Copy the full SHA 7ab27c5View commit details -
Configuration menu - View commit details
-
Copy full SHA for af723d5 - Browse repository at this point
Copy the full SHA af723d5View commit details -
Configuration menu - View commit details
-
Copy full SHA for bf13888 - Browse repository at this point
Copy the full SHA bf13888View commit details -
Begin multiprocessing misadventure
There is no per-instance state involved in USE_SHELL, so pickling is far less directly relevant than usual to multiprocessing: the spawn and forkserver methods will not preserve a subsequently changed attribute value unless side effects of loading a module (or other unpickling of a function or its arguments that are submitted to run on a worker subprocess) causes it to run again; the fork method will. This will be (automatically) the same with any combination of metaclasses, properties, and custom descriptors as in the more straightforward case of a simple class attribute. Subtleties arise in the code that uses GitPython and multiprocessing, but should not arise unintentionally from the change in implementation of USE_SHELL done to add deprecation warnings, except possibly with respect to whether warnings will be repeated in worker processes, which is less important than whether the actual state is preserved.
Configuration menu - View commit details
-
Copy full SHA for 602de0c - Browse repository at this point
Copy the full SHA 602de0cView commit details -
Configuration menu - View commit details
-
Copy full SHA for d4b50c9 - Browse repository at this point
Copy the full SHA d4b50c9View commit details -
Configuration menu - View commit details
-
Copy full SHA for 02c2f00 - Browse repository at this point
Copy the full SHA 02c2f00View commit details -
Configuration menu - View commit details
-
Copy full SHA for 46df79f - Browse repository at this point
Copy the full SHA 46df79fView commit details -
Configuration menu - View commit details
-
Copy full SHA for 40ed842 - Browse repository at this point
Copy the full SHA 40ed842View commit details -
Configuration menu - View commit details
-
Copy full SHA for 6a35261 - Browse repository at this point
Copy the full SHA 6a35261View commit details -
Configuration menu - View commit details
-
Copy full SHA for e725c82 - Browse repository at this point
Copy the full SHA e725c82View commit details -
Add
type: ignore
in test that we can't set USE_SHELL on instancesAs with other `type: ignore` comments in the deprecation tests, mypy will detect if it is superfluous (provided warn_unused_ignores is set to true), thereby enforcing that such code is a static type error.
Configuration menu - View commit details
-
Copy full SHA for 436bcaa - Browse repository at this point
Copy the full SHA 436bcaaView commit details -
Configuration menu - View commit details
-
Copy full SHA for febda6f - Browse repository at this point
Copy the full SHA febda6fView commit details -
Test that Git.execute's own read of USE_SHELL does not warn
+ Use simplefilter where we can (separate from this test).
Configuration menu - View commit details
-
Copy full SHA for 4037108 - Browse repository at this point
Copy the full SHA 4037108View commit details -
Suppress type errors in restore_use_shell_state _USE_SHELL branches
These conditional branches are kept so alternative implementations can be examined, including if they need to be investigated to satisfy some future requirement. But to be unittest.mock.patch patchable, the approaches that would have a _USE_SHELL backing attribute would be difficult to implement in a straightforward way, which seems not to be needed or justified at this time. Since that is not anticipated (except as an intermediate step in development), these suppressions make sense, and they will also be reported by mypy if the implementation changes to benefit from them (so long as it is configured with warn_unused_ignores set to true).
Configuration menu - View commit details
-
Copy full SHA for 0e311bf - Browse repository at this point
Copy the full SHA 0e311bfView commit details -
Configuration menu - View commit details
-
Copy full SHA for df4c5c0 - Browse repository at this point
Copy the full SHA df4c5c0View commit details -
Issue warnings whenever Git.USE_SHELL is accessed
With a special message when it is assigned a True value, which is the dangerous use that underlies its deprecation. The warnings are all DeprecationWarning.
Configuration menu - View commit details
-
Copy full SHA for d38e721 - Browse repository at this point
Copy the full SHA d38e721View commit details -
Implement instance USE_SHELL lookup in __getattr__
This is with the intention of making it so that Git.USE_SHELL is unittest.mock.patch patchable. However, while this moves in the right direction, it can't be done this way, as the attribute is found to be absent on the class, so when unittest.mock.patch unpatches, it tries to delete the attribute it has set, which fails due to the metaclass's USE_SHELL instance property having no deleter.
Configuration menu - View commit details
-
Copy full SHA for 05de5c0 - Browse repository at this point
Copy the full SHA 05de5c0View commit details -
Have USE_SHELL warn but work like normal via super()
This reimplements Git.USE_SHELL with no properties or descriptors. The metaclass is still needed, but instad of defining properties it defines __getattribute__ and __setattr__, which check for USE_SHELL and warn, then invoke the default attribute access via super(). Likewise, in the Git class itself, a __getatttribute__ override is introduced (not to be confused with __getattr__, which was already present and handles attribute access when an attribute is otherwise absent, unlike __getattribute__ which is always used). This checks for reading USE_SHELL on an instance and warns, then invokes the default attribute access via super(). Advantages: - Git.USE_SHELL is again unittest.mock.patch patchable. - AttributeError messages are automatically as before. - It effectively is a simple attribute, yet with warning, so other unanticipated ways of accessing it may be less likely to break. - The code is simpler, cleaner, and clearer. There is some overhead, but it is small, especially compared to a subprocess invocation as is done for performing most git operations. However, this does introduce disadvantages that must be addressed: - Although attribute access on Git instances was already highly dynamic, as "methods" are synthesized for git subcommands, this was and is not the case for the Git class itself, whose attributes remain exactly those that can be inferred without considering the existence of __getattribute__ and __setattr__ on the metaclass. So static type checkers need to be prevented from accounting for those metaclass methods in a way that causes them to infer that arbitrary class attribute access is allowed. - The occurrence of Git.USE_SHELL in the Git.execute method (where the USE_SHELL attribute is actually examined) should be changed so it does not itself issue DeprecationWarning (it is not enough that by default a DeprecationWarning from there isn't displayed).
Configuration menu - View commit details
-
Copy full SHA for 04eb09c - Browse repository at this point
Copy the full SHA 04eb09cView commit details -
Keep mypy from thinking Git has arbitrary class attributes
The existence of __getattr__ or __getattribute__ on a class causes static type checkers like mypy to stop inferring that reads of unrecognized instance attributes are static type errors. When the class is a metaclass, this causes static type checkers to stop inferring that reads of unrecognized class attributes, on classes that use (i.e., that have as their type) the metaclass, are static type errors. The Git class itself defines __getattr__ and __getattribute__, but Git objects' instance attributes really are dynamically synthesized (in __getattr__). However, class attributes of Git are not dynamic, even though _GitMeta defines __getattribute__. Therefore, something special is needed so mypy infers nothing about Git class attributes from the existence of _GitMeta.__getattribute__. This takes the same approach as taken to the analogous problem with __getattr__ at module level, defining __getattribute__ with a different name first and then assigning that to __getattribute__ under an `if not TYPE_CHECKING:`. (Allowing static type checkers to see the original definition allows them to find some kinds of type errors in the body of the method, which is why the method isn't just defined in the `if not TYPE_CHECKING`.) Although it may not currently be necessary to hide __setattr__ too, the same is done with it in _GitMeta as well. The reason is that the intent may otherwise be subtly amgiguous to human readers and maybe future tools: when __setattr__ exists, the effect of setting a class attribute that did not previously exist is in principle unclear, and might not make the attribute readble. (I am unsure if this is the right choice; either approach seems reasonable.)
Configuration menu - View commit details
-
Copy full SHA for c6f518b - Browse repository at this point
Copy the full SHA c6f518bView commit details -
Configuration menu - View commit details
-
Copy full SHA for c5d5b16 - Browse repository at this point
Copy the full SHA c5d5b16View commit details -
Read USE_SHELL in Git.execute without DeprecationWarning
This changes how Git.execute itself accesses the USE_SHELL attribute, so that its own read of it does not issue a warning.
Configuration menu - View commit details
-
Copy full SHA for 84bf2ca - Browse repository at this point
Copy the full SHA 84bf2caView commit details -
Configuration menu - View commit details
-
Copy full SHA for 5bef7ed - Browse repository at this point
Copy the full SHA 5bef7edView commit details -
Hide
del util
from type checkersEven though current type checkers don't seem to need it. As noted in dffa930, it appears neither mypy nor pyright currently needs `del util` to be guarded by `if not TYPE_CHECKING:` -- they currently treat util as bound even without it, even with `del util` present. It is not obvious that this is the best type checker behavior or that type checkers will continue to behave this way. (In addition, it may help humans for all statements whose effects are intended not to be considered for purposes of static typing to be guarded by `if not TYPE_CHECKING:`.) So this guards the deletion of util the same as the binding of __getattr__. This also moves, clarifies, and expands the commented explanations of what is going on.
Configuration menu - View commit details
-
Copy full SHA for 3da47c2 - Browse repository at this point
Copy the full SHA 3da47c2View commit details -
Expand USE_SHELL docstring; clarify a test usage
In the USE_SHELL docstring: - Restore the older wording "when executing git commands" rather than "to execute git commands". I've realized that longer phrase, which dates back to the introduction of USE_SHELL in 1c2dd54, is clearer, because readers less familiar with GitPython's overall design and operation will still not be misled into thinking USE_SHELL ever affects whether GitPython uses an external git command, versus some other mechanism, to do something. - Give some more information about why USE_SHELL = True is unsafe (though further revision or clarification may be called for). - Note some non-obvious aspects of USE_SHELL, that some of the way it interacts with other aspects of GitPython is not part of what is or has been documented about it, and in practice changes over time. The first example relates to gitpython-developers#1792; the second may help users understand why code that enables USE_SHELL on Windows, in addition to being unsafe there, often breaks immediately on other platforms; the third is included so the warnings in the expanded docstring are not interpreted as a new commitment that any shell syntax that may have a *desired* effect in some application will continue to have the same effect in the future. - Cover a second application that might lead, or have led, to setting USE_SHELL to True, and explain what to do instead. In test_successful_default_refresh_invalidates_cached_version_info: - Rewrite the commented explanation of a special variant of that second application, where the usual easy alternatives are not used because part of the goal of the test is to check a "default" scenario that does not include either of them. This better explains why that choice is made in the test, and also hopefully will prevent anyone from thinking that test is a model for another situation (which, as noted, is unlikely to be the case even in unit tests).
Configuration menu - View commit details
-
Copy full SHA for bdabb21 - Browse repository at this point
Copy the full SHA bdabb21View commit details -
Explain the approach in test.deprecation to static checking
And why this increases the importance of the warn_unused_ignored mypy configuration option.
Configuration menu - View commit details
-
Copy full SHA for b51b080 - Browse repository at this point
Copy the full SHA b51b080View commit details
Commits on Mar 30, 2024
-
Make test.performance.lib docstring more specific
To distinguish it from the more general test.lib as well as from the forthcoming test.deprecation.lib.
Configuration menu - View commit details
-
Copy full SHA for 7cd3aa9 - Browse repository at this point
Copy the full SHA 7cd3aa9View commit details -
Make/use test.deprecation.lib; abandon idea to filter by module
This creates a module test.deprecation.lib in the test suite for a couple general helpers used in deprecation tests, one of which is now used in two of the test modules and the other of which happens only to be used in one but is concepually related to the first. The assert_no_deprecation_warning context manager function fixes two different flawed approaches to that, which were in use earlier: - In test_basic, only DeprecationWarning and not the less significant PendingDeprecationWarning had been covere, which it should, at least for symmetry, since pytest.deprecated_call() treats it like DeprecationWarning. There was also a comment expressing a plan to make it filter only for warnings originating from GitPython, which was a flawed idea, as detailed below. - In test_cmd_git, the flawed idea of attempting to filter only for warnings originating from GitPython was implemented. The problem with this is that it is heavily affected by stacklevel and its interaction with the pattern of calls through which the warning arises: warnings could actually emanate from code in GitPython's git module, but be registered as having come from test code, a callback in gitdb, smmap, or the standard library, or even the pytest test runner. Some of these are more likely than others, but all are possible especially considering the possibility of a bug in the value passed to warning.warn as stacklevel. (It may be valuable for such bugs to cause tests to fail, but they should not cause otherwise failing tests to wrongly pass.) It is probably feasible to implement such filtering successfully, but I don't think it's worthwhile for the small reduction in likelihood of failing later on an unrealted DeprecationWarning from somewhere else, especially considering that GitPython's main dependencies are so minimal.
Configuration menu - View commit details
-
Copy full SHA for cf2576e - Browse repository at this point
Copy the full SHA cf2576eView commit details
Commits on Mar 31, 2024
-
Configuration menu - View commit details
-
Copy full SHA for f92f4c3 - Browse repository at this point
Copy the full SHA f92f4c3View commit details -
Configuration menu - View commit details
-
Copy full SHA for 8327b45 - Browse repository at this point
Copy the full SHA 8327b45View commit details -
Configuration menu - View commit details
-
Copy full SHA for f6060df - Browse repository at this point
Copy the full SHA f6060dfView commit details