Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Remove unittest code from non-test code #790

Open
mwatts15 opened this issue Aug 23, 2018 · 1 comment
Open

Remove unittest code from non-test code #790

mwatts15 opened this issue Aug 23, 2018 · 1 comment
Labels
acknowledged

Comments

@mwatts15
Copy link

I see these imports of unittest in

I generally think that test code should not be built into the business logic. Is there a reason why using a skipIf decorator is insufficient?
@Byron
Copy link
Member

Byron commented Oct 14, 2018 •
edited
Loading

Good catch, I absolutely agree! Even though I am pretty sure there was a very good reason to do that at some point in time. With the current state of windows test, for which these dependencies seemingly have been introduced. Maybe @ankostis can provide more context, who did magnificent work a while ago to get windows back into shape.

@EliahKagan EliahKagan mentioned this issue Sep 26, 2023
Merged
This was referenced Oct 9, 2023
Closed
Closed
Merged
Merged
@EliahKagan EliahKagan mentioned this issue Nov 3, 2023
Closed
@EliahKagan EliahKagan mentioned this issue Nov 14, 2023
Merged
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Nov 14, 2023
@EliahKagan
List all non-passing tests in test summaries
359116b 
This changes the default in pyproject.toml so that pytest lists
a line for each non-passing test at the end of a run, showing the
test name and, where available, condensed information about the
status, such as the "reason" argument for an xfailing or skipped
test.

Previously only failed and errored tests were listed in the
summary. Now skipped, xfailed, and xpassed tests are listed too.
The benefit is in keeping track of the status of tests. Although
showing the full failure output with stack trace and relevant code
under test would be too distracting for tests marked xfail, it is
valuable to not merely run those tests but be able to see a line
showing their names and statuses. Likewise, a number of tests are
currently marked skipped, and while some of them are skipped on a
particular platform because they don't make sense to run on that
platform, a number of others are skipped by raising SkipTest in
response to a failure condition on Windows. (Those consist mostly
of the tests skipped as a result of code discussed in gitpython-developers#790.)

This also has the more specific benefit of making it easier to mark
tests as xfail in order to add CI jobs for native Windows, and more
importantly to allow information about their status to later be
used to understand and fix bugs on Windows.)
@EliahKagan EliahKagan mentioned this issue Nov 14, 2023
Merged
@EliahKagan EliahKagan mentioned this issue Nov 28, 2023
Merged
marioaag pushed a commit to marioaag/GitPython that referenced this issue Nov 28, 2023
@EliahKagan
List all non-passing tests in test summaries
07f3641 
This changes the default in pyproject.toml so that pytest lists
a line for each non-passing test at the end of a run, showing the
test name and, where available, condensed information about the
status, such as the "reason" argument for an xfailing or skipped
test.

Previously only failed and errored tests were listed in the
summary. Now skipped, xfailed, and xpassed tests are listed too.
The benefit is in keeping track of the status of tests. Although
showing the full failure output with stack trace and relevant code
under test would be too distracting for tests marked xfail, it is
valuable to not merely run those tests but be able to see a line
showing their names and statuses. Likewise, a number of tests are
currently marked skipped, and while some of them are skipped on a
particular platform because they don't make sense to run on that
platform, a number of others are skipped by raising SkipTest in
response to a failure condition on Windows. (Those consist mostly
of the tests skipped as a result of code discussed in gitpython-developers#790.)

This also has the more specific benefit of making it easier to mark
tests as xfail in order to add CI jobs for native Windows, and more
importantly to allow information about their status to later be
used to understand and fix bugs on Windows.)
This was referenced Dec 8, 2023
Merged
Merged
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Dec 19, 2023
@EliahKagan
Run test_env_vars_for_windows_tests only on Windows
6a8ed70 
This skips the tests of how the HIDE_WINDOWS_KNOWN_ERRORS and
HIDE_WINDOWS_FREEZE_ERRORS environment variables affect the
same-named attributes of git.util, except when testing on Windows.

These are parsed only to ever set a True value on Windows, but
checking that this is the case is less important ever since
git.util.rmtree was changed to not check HIDE_WINDOWS_KNOWN_ERRORS
on other systems (and this is covered in other tests).

Setting the variables to True on non-Windows systems would still
have a bad effect on the tests themselves, some of which use them
as skip or xfail conditions separate from the skipping logic in
git.util.rmtree. However, this is effectively using them as part of
the test suite (which they were initially meant for and which they
may eventually go back to being, for gitpython-developers#790), where they would not
ordinarily have tests.

The benefit and motivation for running these tests only on Windows
is that the tests can be simplified, so that their parameter sets
are no longer confusing. That change is also made here.
@EliahKagan EliahKagan mentioned this issue Dec 19, 2023
Merged
EliahKagan added a commit to EliahKagan/GitPython that referenced this issue Dec 22, 2023
@EliahKagan
Convert constant and attribute comments to docstrings
94a85d1 
These are "non-reified docstrings" as described in gitpython-developers#1734. They are
not made part of the data model, but most editors will display
them, including on symbols present in code of other projects that
use the GitPython library. A number of these have already been
added, but this adds what will probably be most of the remaining
ones.

For the most part, this doesn't create documentation where there
wasn't any, but instead converts existing comments to "docstrings."
In a handful of cases, they are expanded, reworded, or a docstring
added.

This also fixes some small style inconsistencies that were missed
in gitpython-developers#1725, and moves a comment that had become inadvertently
displaced due to autoformatting to the item it was meant for.

The major omission here is HIDE_WINDOWS_KNOWN_ERRORS and
HIDE_WINDOWS_FREEZE_ERRORS. This doesn't convert the comments above
them to "docstrings," for a few reasons. They are not specific to
either of the symbols, they are oriented toward considerations that
are not really relevant except when developing GitPython itself,
and they are out of date. Also, because HIDE_WINDOWS_KNOWN_ERRORS
is listed in __all__, increasing the level of documentation for it
might be taken as a committment to preserve some aspect of its
current behavior, which could interfere with progress on gitpython-developers#790. So
I've kept those comments as comments, and unchanged, for now.
@EliahKagan EliahKagan mentioned this issue Mar 30, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
acknowledged
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Byron @mwatts15