Solve typing issues, improve coverage

conftest.py

@@ -149,6 +149,8 @@ def pytest_runtest_setup(item):
         "botocore",
         "boto3",
         "mitmproxy",
+        "keyring",
+        "dotenv",
     ]
 
     for module in optional_deps:

docs/topics/settings.rst

@@ -263,7 +263,7 @@ JSON object with an ``"env"`` key:
 If the environment variable is not set, ``getsecret()`` logs a warning and the
 default setting value is used instead.
 
-To read the secret from the system keyring (requires the keyring_ package),
+To read the secret from the system keyring (requires keyring_ >= 21.7.0),
 use a ``"keyring"`` key.  The value can be a username string — in which case
 the service name defaults to ``"scrapy"`` — or an object for full control:
 
@@ -813,7 +813,7 @@ each crawl (via :class:`~scrapy.crawler.Crawler`). Relative paths are resolved
 from the current working directory. If the file does not exist the setting is
 silently ignored.
 
-Loading a :file:`.env` file requires the python-dotenv_ package::
+Loading a :file:`.env` file requires python-dotenv_ >= 0.10.2::
 
     pip install python-dotenv
 

pyproject.toml

@@ -269,6 +269,8 @@ markers = [
     "requires_botocore: marks tests that need botocore (but not boto3)",
     "requires_boto3: marks tests that need botocore and boto3",
     "requires_mitmproxy: marks tests that need mitmproxy",
+    "requires_keyring: marks tests that need keyring",
+    "requires_dotenv: marks tests that need python-dotenv",
     "requires_internet: marks tests that need real Internet access",
 ]
 filterwarnings = [

scrapy/core/downloader/handlers/s3.py

@@ -37,7 +37,9 @@ class S3DownloadHandler(BaseDownloadHandler):
             # botocore.auth.BaseSigner doesn't have an __init__() with args, only subclasses do
             self._signer = SignerCls(  # type: ignore[call-arg]
                 botocore.credentials.Credentials(
-                    aws_access_key_id, aws_secret_access_key, aws_session_token
+                    aws_access_key_id,
+                    aws_secret_access_key,
+                    aws_session_token,  # type: ignore[arg-type]
                 )
             )
 

scrapy/pipelines/files.py

@@ -153,9 +153,9 @@ class FSFilesStore:
 
 
 class S3FilesStore:
-    AWS_ACCESS_KEY_ID = None
-    AWS_SECRET_ACCESS_KEY = None
-    AWS_SESSION_TOKEN = None
+    AWS_ACCESS_KEY_ID: str | None = None
+    AWS_SECRET_ACCESS_KEY: str | None = None
+    AWS_SESSION_TOKEN: str | None = None
     AWS_ENDPOINT_URL = None
     AWS_REGION_NAME = None
     AWS_USE_SSL = None

tests/test_utils_secrets.py

@@ -230,8 +230,8 @@ class TestLoadDotenv:
         _load_dotenv(path=absent)
         assert dict(__import__("os").environ) == before
 
+    @pytest.mark.requires_dotenv
     def test_loads_variables(self, tmp_path):
-        pytest.importorskip("dotenv")
         dotenv_file = tmp_path / ".env"
         dotenv_file.write_text("SCRAPY_TEST_SECRET_VAR=hello\n")
         with mock.patch.dict("os.environ", {}, clear=False):
@@ -241,8 +241,8 @@ class TestLoadDotenv:
             assert __import__("os").environ.get("SCRAPY_TEST_SECRET_VAR") == "hello"
             __import__("os").environ.pop("SCRAPY_TEST_SECRET_VAR", None)
 
+    @pytest.mark.requires_dotenv
     def test_real_env_wins_over_dotenv(self, tmp_path):
-        pytest.importorskip("dotenv")
         dotenv_file = tmp_path / ".env"
         dotenv_file.write_text("SCRAPY_TEST_OVERRIDE_VAR=from-file\n")
         with mock.patch.dict(
@@ -251,8 +251,8 @@ class TestLoadDotenv:
             _load_dotenv(path=str(dotenv_file), override=False)
             assert __import__("os").environ["SCRAPY_TEST_OVERRIDE_VAR"] == "from-env"
 
+    @pytest.mark.requires_dotenv
     def test_override_true_replaces_env(self, tmp_path):
-        pytest.importorskip("dotenv")
         dotenv_file = tmp_path / ".env"
         dotenv_file.write_text("SCRAPY_TEST_OVERRIDE_VAR=from-file\n")
         with mock.patch.dict(
@@ -261,3 +261,47 @@ class TestLoadDotenv:
             _load_dotenv(path=str(dotenv_file), override=True)
             assert __import__("os").environ["SCRAPY_TEST_OVERRIDE_VAR"] == "from-file"
             __import__("os").environ.pop("SCRAPY_TEST_OVERRIDE_VAR", None)
+
+
+@pytest.mark.requires_keyring
+class TestResolveSecretKeyringIntegration:
+    """Integration tests that exercise the real keyring library."""
+
+    def test_get_password_returns_value(self):
+        import keyring  # noqa: PLC0415
+        import keyring.backend  # noqa: PLC0415
+        import keyring.backends.null  # noqa: PLC0415
+
+        class _InMemoryKeyring(keyring.backend.KeyringBackend):
+            priority = 1
+            _store: dict[tuple[str, str], str] = {}
+
+            def get_password(self, service, username):
+                return self._store.get((service, username))
+
+            def set_password(self, service, username, password):
+                self._store[(service, username)] = password
+
+            def delete_password(self, service, username):
+                self._store.pop((service, username), None)
+
+        backend = _InMemoryKeyring()
+        backend.set_password("scrapy", "my-account", "real-secret")
+        original = keyring.get_keyring()
+        try:
+            keyring.set_keyring(backend)
+            assert resolve_secret({"keyring": "my-account"}) == "real-secret"
+        finally:
+            keyring.set_keyring(original)
+
+    def test_missing_entry_raises_key_error(self):
+        import keyring  # noqa: PLC0415
+        import keyring.backends.null  # noqa: PLC0415
+
+        original = keyring.get_keyring()
+        try:
+            keyring.set_keyring(keyring.backends.null.Keyring())
+            with pytest.raises(KeyError):
+                resolve_secret({"keyring": "nonexistent"})
+        finally:
+            keyring.set_keyring(original)

tox.ini

@@ -47,6 +47,8 @@ deps =
     mypy==2.1.0
     typing-extensions==4.15.0
     Pillow==12.2.0
+    keyring==22.3.0
+    python-dotenv==0.10.2
     Protego==0.6.0
     Twisted==26.4.0
     attrs==26.1.0
@@ -143,6 +145,8 @@ basepython = python3
 deps =
     {[testenv]deps}
     Pillow
+    keyring
+    python-dotenv
     Twisted[http2]
     boto3
     bpython  # optional for shell wrapper tests
@@ -160,6 +164,8 @@ basepython = {[pinned]basepython}
 deps =
     {[pinned]deps}
     Pillow==8.3.2
+    keyring==21.7.0
+    python-dotenv==0.10.2
     Twisted[http2]==21.7.0
     boto3==1.20.0
     bpython==0.7.1