GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Don't suppress pytest warning summaries by EliahKagan · Pull Request #1892 · gitpython-developers/GitPython

Conversation

@EliahKagan

@EliahKagan EliahKagan commented Mar 31, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

This removes the --disable-warnings option from those specified in pyproject.toml for pytest to use automatically.

I originally tried this in the hope that it would show a warning for misspelled keyword arguments passed when constructing pytest marks (which should not be confused with misspelling the mark itself, which is something pytest can warn for). That bug (#1893) was discovered in #1888. This turns out not to be useful for that at all, so I'm opening this PR separate from anything related to that.

Nonetheless, there doesn't seem to be a significant disadvantage to showing this output, which is not easy to mistake for errors or test failures, and which might potentially help catch other kind of cases where a misspelling that intuitively feels like it would lead to an error instead produces a major change in semantics for the tests, in addition to other problems.

There are also some resource-related warnings that may help point the way to debugging things, especially in future examinations of how GitPython interacts with garbage collection on Windows.

Therefore, I think it makes sense to allow these warnings summaries to be shown.

I believe this does not worsen #1891, because these do not look like errors. I suspect it might very slightly mitigate it in some cases, even, since the warnings section near the xfailures section makes it easier to notice that these sections are not just for reporting errors and actual test failures. However, I definitely do not advocate this on the basis of that possibility; in my opinion, allowing the warning summaries to be shown is justified in spite of how it increases the overall length and number of sections of the test output, not because of that.

Deprecation warnings are separately suppressed and this does not change that, though perhaps it ought to, since eventually all areas in the tests that allow a deprecation warning to emanate from them should probably be marked for that specifically. This is also separate from how the new tests in test/deprecation (added in #1886) that check for the presence or absence of deprecation warnings work: those tests will produce hard failures if their expectations are not met, regardless of the changes here or to whether deprecation warnings elsewhere are allowed into the warnings summaries.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@EliahKagan
Don't pass --disable-warnings to pytest
f18df8e
@EliahKagan
EliahKagan force-pushed the pytest-warnings branch from 3761721 to f18df8e Compare March 31, 2024 23:52
@EliahKagan
EliahKagan marked this pull request as ready for review March 31, 2024 23:52
@Byron

Byron commented Apr 2, 2024

Copy link
Copy Markdown
Member

Thanks a lot! Anything on the way to improving on #1891 is appreciated.

EliahKagan reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@Byron
Byron merged commit f8cad58 into gitpython-developers:main Apr 2, 2024
@EliahKagan
EliahKagan deleted the pytest-warnings branch April 2, 2024 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@EliahKagan @Byron
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.