dwreeves commented on code in PR #62378:
URL: https://github.com/apache/airflow/pull/62378#discussion_r2848722949
##########
providers/snowflake/tests/unit/snowflake/hooks/test_snowflake.py:
##########
@@ -372,6 +372,47 @@ def
test_hook_should_support_prepare_basic_conn_params_and_uri(
assert SnowflakeHook(snowflake_conn_id="test_conn").get_uri() ==
expected_uri
assert
SnowflakeHook(snowflake_conn_id="test_conn")._get_conn_params() ==
expected_conn_params
Review Comment:
Actually, similar to what I did with the other test, I am not going to mock
out `b64decode`. My justification is similar to the reasons I gave in the other
comment. https://github.com/apache/airflow/pull/62378#discussion_r2847663977
Actually, I have one more justification to add: mocking is annoying because
we need to deal with the `MagicMock` object internally, i.e. by hardcoding in
the `return_value`. At which point we're trending closer to just testing the
test rather than testing the code.
What I did instead was assert that the returned `private_key` is correct:
```python
def test_get_conn_params_should_support_private_auth_in_connection(
self, base64_encoded_encrypted_private_key: str,
encrypted_temporary_private_key: Path
):
private_key_content = encrypted_temporary_private_key.read_text()
p_key = serialization.load_pem_private_key(
private_key_content.encode(),
password=_PASSWORD.encode(),
backend=default_backend(),
)
pkb = p_key.private_bytes(
encoding=serialization.Encoding.DER,
format=serialization.PrivateFormat.PKCS8,
encryption_algorithm=serialization.NoEncryption(),
)
connection_kwargs: Any = {
**BASE_CONNECTION_KWARGS,
"password": _PASSWORD,
"extra": {
"database": "db",
"account": "airflow",
"warehouse": "af_wh",
"region": "af_region",
"role": "af_role",
"private_key_content": base64_encoded_encrypted_private_key,
},
}
with (
mock.patch.dict("os.environ",
AIRFLOW_CONN_TEST_CONN=Connection(**connection_kwargs).get_uri())
):
conn_params =
SnowflakeHook(snowflake_conn_id="test_conn")._get_conn_params()
assert "private_key" in conn_params
assert conn_params["private_key"] == pkb
```
The original code does not do this; all it asserts is `"private_key" in
conn_params`.
This is sufficient to ensure that the private key is what we expect it to
be: it takes in the b64encoded value, and we ensure it is eventually a valid
private key.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
