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.

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

gh-128770: raise warnings as errors in test suite - except for test_socket which still logs warnings, and internal test warnings that are now logged #128973

Open
wants to merge 26 commits into
base: main
Choose a base branch
from

Conversation

graingert
Copy link
Contributor

@graingert graingert commented Jan 18, 2025

@graingert
Copy link
Contributor Author

@hugovk can I get a run on the buildbots for this PR?

@hugovk
Copy link
Member

hugovk commented Jan 18, 2025

Certainly!

Let's check if you can do it yourself: either apply one of the test-with-buildbots or test-with-refleak-buildbots labels, or add a comment like ! buildbot optional-regex (but no space between exclamation mark and buildbot) to run a subset.

@graingert graingert added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 18, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @graingert for commit c06c3db 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 18, 2025
@graingert graingert added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 18, 2025
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @graingert for commit 7123093 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 18, 2025
@graingert
Copy link
Contributor Author

@hugovk do you know where I can see the status?

@hugovk
Copy link
Member

hugovk commented Jan 18, 2025

You can click the "Details" links in the status checks:

image image

Some of those failing ones were already failing for previous builds, so aren't necessarily caused by this PR.

@graingert
Copy link
Contributor Author

You can click the "Details" links in the status checks:
image image

Some of those failing ones were already failing for previous builds, so aren't necessarily caused by this PR.

Ah those hadn't shown up yet

@graingert graingert marked this pull request as ready for review January 18, 2025 22:48
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @graingert for commit 36bba4e 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 20, 2025
@graingert graingert requested a review from picnixz January 20, 2025 10:38
Co-Authored-By: Bénédikt Tran <[email protected]>
Lib/test/test_support.py Outdated Show resolved Hide resolved
Lib/test/test_support.py Outdated Show resolved Hide resolved
Lib/test/test_support.py Outdated Show resolved Hide resolved
Lib/test/test_support.py Outdated Show resolved Hide resolved
Lib/test/test_support.py Outdated Show resolved Hide resolved
Lib/test/test_support.py Outdated Show resolved Hide resolved
Lib/test/test_support.py Outdated Show resolved Hide resolved
Lib/test/test_support.py Show resolved Hide resolved
Lib/test/test_socket.py Outdated Show resolved Hide resolved
Lib/test/test_socket.py Outdated Show resolved Hide resolved
Lib/test/test_support.py Outdated Show resolved Hide resolved
Lib/test/test_support.py Outdated Show resolved Hide resolved
@graingert graingert requested a review from picnixz January 21, 2025 09:41
@picnixz
Copy link
Member

picnixz commented Jan 21, 2025

I'll let Hugo have a look as well!

LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.
"""
Copy link
Member

Choose a reason for hiding this comment

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

About the licence, in general this seems fine to use/modify the code and credit it, but maybe it should be mentioned in https://docs.python.org/3/license.html instead? Let's ask if @encukou knows.

Copy link
Member

Choose a reason for hiding this comment

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

That is where we put the licences (see e.g. https://docs.python.org/3/license.html#test-epoll)
But I don't know much about incorporating code where not all authors signed the CLA.

It seems caplog is derived from pytest-catchlog which has a different copyright section.

Copy link
Contributor Author

@graingert graingert Jan 25, 2025

Choose a reason for hiding this comment

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

how about if I just pare it down to the bare minimum for the tests to pass:

class LogCaptureHandler(logging.StreamHandler):
    def __init__(self):
        super().__init__(io.StringIO())
        self.records = []

    def emit(self, record) -> None:
        self.records.append(record)
        super().emit(record)

Copy link
Contributor Author

@graingert graingert Jan 25, 2025

Choose a reason for hiding this comment

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

is the above small enough to not be a concern for copyright?

@graingert
Copy link
Contributor Author

I'm having trouble with the new warnings introduced #129287

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants