mirror of
https://github.com/certbot/certbot.git
synced 2026-07-28 08:45:19 +02:00
Use python warning filters from pytest.ini during integration tests (#10602)
Fixes https://github.com/certbot/certbot/issues/10180. So first of all, the core issue here is that [pyca deliberately chose](https://github.com/pyca/cryptography/blob/ec80c1c2894320d30fd674ea2c6103d91b4e777e/src/cryptography/utils.py#L15-L18) to override the default python functionality and make deprecation warnings appear by default. This isn't common. If they'd actually used a `DeprecationWarning`, it wouldn't have shown up to users, at least. That being said, we should still try to catch it, as we do in fact want to know about deprecation warnings for our own updates. To do that, this PR searches upwards for a `pytest.ini` file from the file's location. If found, it reads the warnings from the file, and passes them using the `PYTHONWARNINGS` env variable. It also explicitly sets warnings to `error` always in case we can't find the `pytest.ini`, and ignores the subsequent unverified-https-on-localhost warning. It also fixes a warning in our test nginx config that seemed reasonable to address. I tested this by adding a temporary warning, which I then removed, but since it turned out there were two other warnings, that wasn't actually necessary. Options I considered and rejected: - Switch from `atexit` to calling `main` directly. To do this, we'd have to switch our `main` function to something like a try-finally. That's complicated by the fact that we call `atexit` from other places in the code. Also, `exc_info` isn't availabe in `finally` while it is in `at_exit`, so it's not as versatile. But mostly if we wanted to do this, we'd have to implement a custom atexit handler, basically, and that seems worse than this option. - Looking into pytest-forked. It's apparently buggy and not being maintained. Not even sure this is what it's for anyway. - Multiple [-W](https://docs.python.org/3/using/cmdline.html#cmdoption-W) options can be given instead of an env variable. The env version seemed cleaner. - More closely mimicking [how pytest finds ini files](https://docs.pytest.org/en/stable/reference/customize.html#finding-the-rootdir). It seemed unnecessary to me. Potential drawbacks: - If we move or rename the `pytest.ini` file and for some reason don't do a reasonable grep for `pytest.ini`, we will no longer catch any additional `ignore`s in there. But imo we're likely to do that grep, and also a missing ignore will then show up when we run the tests.
This commit is contained in:
@@ -1,7 +1,6 @@
|
||||
# -*- coding: utf-8 -*-
|
||||
"""General purpose nginx test configuration generator."""
|
||||
import atexit
|
||||
import getpass
|
||||
import importlib.resources
|
||||
from contextlib import ExitStack
|
||||
from typing import Optional
|
||||
@@ -49,7 +48,6 @@ error_log {nginx_root}/error.log;
|
||||
# The pidfile will be written to /var/run unless this is set.
|
||||
pid {nginx_root}/nginx.pid;
|
||||
|
||||
user {user};
|
||||
worker_processes 1;
|
||||
|
||||
events {{
|
||||
@@ -136,7 +134,7 @@ http {{
|
||||
ssl_certificate_key {key_path};
|
||||
}}
|
||||
}}
|
||||
'''.format(nginx_root=nginx_root, nginx_webroot=nginx_webroot, user=getpass.getuser(),
|
||||
'''.format(nginx_root=nginx_root, nginx_webroot=nginx_webroot,
|
||||
http_port=http_port, https_port=https_port, other_port=other_port,
|
||||
default_server='default_server' if default_server else '', wtf_prefix=wtf_prefix,
|
||||
key_path=key_path, cert_path=cert_path)
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
#!/usr/bin/env python
|
||||
"""Module to call certbot in test mode"""
|
||||
|
||||
import configparser
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
@@ -77,6 +78,38 @@ def _prepare_environ(workspace: str) -> dict[str, str]:
|
||||
]
|
||||
new_environ['PYTHONPATH'] = ':'.join(python_paths)
|
||||
|
||||
# Pytest finds its ini file by doing the following:
|
||||
# Determine the common ancestor directory for the specified args that are recognised as paths
|
||||
# that exist in the file system. If no such paths are found, the common ancestor directory is
|
||||
# set to the current working directory.
|
||||
#
|
||||
# Look for pytest.toml, .pytest.toml, pytest.ini, .pytest.ini, pyproject.toml, tox.ini, and
|
||||
# setup.cfg files in the ancestor directory and upwards. If one is matched, it becomes the
|
||||
# configfile and its directory becomes the rootdir.
|
||||
# source: https://docs.pytest.org/en/stable/reference/customize.html#finding-the-rootdir
|
||||
#
|
||||
# Certbot's is located in its root directory. Let's just walk up the tree from this file.
|
||||
# In case we can't find it, we still do want to error, so add a default. It's fine to have
|
||||
# error in there twice. Also, ignore the unverified HTTPS request specifically for this call.
|
||||
warning_filters: list[str]= [
|
||||
'error',
|
||||
"ignore:Unverified HTTPS request is being made to host 'localhost'",
|
||||
]
|
||||
base_path: str = os.path.realpath(__file__)
|
||||
while base_path != os.sep:
|
||||
ini_loc: str = os.path.join(base_path, 'pytest.ini')
|
||||
if os.path.exists(ini_loc):
|
||||
ini_config: configparser.ConfigParser = configparser.ConfigParser()
|
||||
ini_config.read(ini_loc)
|
||||
if ini_config is not None:
|
||||
ini_filters = ini_config.get('pytest', 'filterwarnings', fallback='')
|
||||
warning_filters.extend(ini_filters.split('\n'))
|
||||
break
|
||||
else:
|
||||
base_path = os.path.dirname(base_path)
|
||||
|
||||
new_environ['PYTHONWARNINGS'] = ','.join(warning_filters)
|
||||
|
||||
return new_environ
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user