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
   ```
   
   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]

Reply via email to