kaxil commented on a change in pull request #12742:
URL: https://github.com/apache/airflow/pull/12742#discussion_r533671859



##########
File path: tests/core/test_configuration.py
##########
@@ -557,6 +557,23 @@ def test_command_from_env(self):
             # the environment variable's echo command
             self.assertEqual(test_cmdenv_conf.get('testcmdenv', 
'notacommand'), 'OK')
 
+    def test_sensitive_config_values(self):

Review comment:
       We don't only write tests for what users directly use. We need to test 
what the expected behaviour of the attribute would be.
   
   > I don't have a strong opinion on this, but I would prefer the tests to 
check the behavior, not just the value in the class attribute. I would be fine 
if we weren't to assume that we have other tests that cover behaviors, and this 
is just a configuration option.
   > 
   > In real life, no user will write the code:
   > 
   > ```python
   > config_parser = AirflowConfigParser()
   > print(config_parser.sensitive_config_values)
   > ```
   > 
   > so this attribute is an internal implementation detail.
   
   




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to