Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions src/pystatis/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,9 +151,26 @@ def config_exists() -> bool:
return config_file.exists()


def setup_credentials() -> None:
"""Setup credentials for all supported databases."""
for db_name in get_supported_db():
def setup_credentials(db_names: list[str] | None = None) -> None:
"""
Setup credentials for all supported databases.

Args:
db_names (list[str]): Names of the databases to setup
"""
if db_names is None:
db_names = get_supported_db()

for db_name in db_names:
# despite this check, we should consider using literals as the type hint for
# db_names.
if db_name not in get_supported_db():
raise KeyError(
f"Provided db_name '{db_name}' no regnized. "
f"Valid options are {get_supported_db()}"
)

for db_name in db_names:
config.set(db_name, "username", _get_user_input(db_name, "username"))
config.set(db_name, "password", _get_user_input(db_name, "password"))
if not db.check_credentials_are_valid(db_name):
Expand Down
19 changes: 19 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,25 @@ def test_setup_credentials(mocker, config_):
assert config_[db_name]["password"] == "test123!"


def test_setup_credentials_with_db_names_subset(mocker, config_):
mocker.patch.object(db, "check_credentials_are_valid", return_value=True)

os.environ["PYSTATIS_GENESIS_API_USERNAME"] = "test"
os.environ["PYSTATIS_GENESIS_API_PASSWORD"] = "test123!"

config.setup_credentials(db_names=["genesis"])

# Only the specified DB should be set
assert config_["genesis"]["username"] == "test"
assert config_["genesis"]["password"] == "test123!"
Comment on lines +86 to +96
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This subset test can still pass if all DBs are configured.

test_setup_credentials above leaves PYSTATIS_ZENSUS_* and PYSTATIS_REGIO_* in os.environ, and this test never asserts those sections stay empty. If setup_credentials(db_names=["genesis"]) regresses to iterating all supported DBs, this test can still go green.

Proposed fix
 def test_setup_credentials_with_db_names_subset(mocker, config_):
     mocker.patch.object(db, "check_credentials_are_valid", return_value=True)
 
     os.environ["PYSTATIS_GENESIS_API_USERNAME"] = "test"
     os.environ["PYSTATIS_GENESIS_API_PASSWORD"] = "test123!"
+    os.environ.pop("PYSTATIS_ZENSUS_API_USERNAME", None)
+    os.environ.pop("PYSTATIS_ZENSUS_API_PASSWORD", None)
+    os.environ.pop("PYSTATIS_REGIO_API_USERNAME", None)
+    os.environ.pop("PYSTATIS_REGIO_API_PASSWORD", None)
 
     config.setup_credentials(db_names=["genesis"])
 
-    # Only the specified DB should be set
     assert config_["genesis"]["username"] == "test"
     assert config_["genesis"]["password"] == "test123!"
+    assert config_["zensus"]["username"] == ""
+    assert config_["zensus"]["password"] == ""
+    assert config_["regio"]["username"] == ""
+    assert config_["regio"]["password"] == ""
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_config.py` around lines 86 - 96, The test
test_setup_credentials_with_db_names_subset can falsely pass if other DB envvars
remain set; update the test so before calling
config.setup_credentials(db_names=["genesis"]) you explicitly clear or unset the
other DB environment variables (e.g. PYSTATIS_ZENSUS_* and PYSTATIS_REGIO_*) or
after calling config.setup_credentials assert that config_ entries for the
non-requested DBs (e.g. "zensus", "regio") have no username/password set; target
the test function test_setup_credentials_with_db_names_subset and the call to
config.setup_credentials to ensure only "genesis" is populated and other DB
sections remain empty.



def test_setup_credentials_invalid_credentials_raises(mocker, config_):
mocker.patch.object(db, "check_credentials_are_valid", return_value=False)

with pytest.raises(KeyError) as _exc_info:
config.setup_credentials(db_names=["regio_s"])
Comment on lines +99 to +103
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

This test never reaches the invalid-credentials branch.

With db_names=["regio_s"], the new guard in src/pystatis/config.py:162-166 raises KeyError before Line 169 can call db.check_credentials_are_valid, so the mock on Line 100 is unused. This currently tests “unsupported DB name”, not “invalid credentials”.

Suggested direction
-def test_setup_credentials_invalid_credentials_raises(mocker, config_):
-    mocker.patch.object(db, "check_credentials_are_valid", return_value=False)
-
-    with pytest.raises(KeyError) as _exc_info:
-        config.setup_credentials(db_names=["regio_s"])
+def test_setup_credentials_invalid_db_name_raises(config_):
+    with pytest.raises(KeyError):
+        config.setup_credentials(db_names=["regio_s"])

Then add a separate test for the actual invalid-credentials path using a supported name such as "regio" and pytest.raises(config.PystatisConfigError).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_setup_credentials_invalid_credentials_raises(mocker, config_):
mocker.patch.object(db, "check_credentials_are_valid", return_value=False)
with pytest.raises(KeyError) as _exc_info:
config.setup_credentials(db_names=["regio_s"])
def test_setup_credentials_invalid_db_name_raises(config_):
with pytest.raises(KeyError):
config.setup_credentials(db_names=["regio_s"])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_config.py` around lines 99 - 103, The test
test_setup_credentials_invalid_credentials_raises currently uses
db_names=["regio_s"] which triggers the unsupported-name guard in
config.setup_credentials and never calls db.check_credentials_are_valid; update
the test to target the invalid-credentials branch by using a supported DB name
like "regio" so the mocker.patch.object(db, "check_credentials_are_valid",
return_value=False) is exercised, and assert the function raises the appropriate
exception (config.PystatisConfigError) instead of KeyError; keep the existing
unsupported-name scenario as a separate test asserting KeyError for "regio_s".


@pytest.mark.parametrize(
"mock_return, check_result",
[
Expand Down