Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid side-effects in conf_vars testing helper #37438

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

Taragolis
Copy link
Contributor

In pytest 8.0.0 the order of tests cases from @pytest.mark.parametrize executed in reversed order. This would be fixed in 8.0.1 (or whatewer version would be next):

This regression in pytest shows that we have some side effects in conf_vars helper,
e.g. if run tests/providers/amazon/aws/log/test_cloudwatch_task_handler.py::TestCloudwatchTaskHandler::test_write_json_logs in pytest 8.0.0
or run it individually it would fail

pytest "tests/providers/amazon/aws/log/test_cloudwatch_task_handler.py::TestCloudwatchTaskHandler::test_write_json_logs[not-set]"


======================================================================================================== test session starts ========================================================================================================
platform darwin -- Python 3.9.10, pytest-8.0.0, pluggy-1.3.0 -- /Users/taragolis/.pyenv/versions/3.9.10/envs/airflow-dev-env-39/bin/python
cachedir: .pytest_cache
rootdir: /Users/taragolis/Projects/common/airflow
configfile: pyproject.toml
plugins: anyio-4.0.0, instafail-0.5.0, forked-1.4.0, httpx-0.21.3, rerunfailures-12.0, timeouts-1.2.1, time-machine-2.13.0, cov-4.1.0, asyncio-0.21.1, mock-3.12.0, aiohttp-1.0.5, requests-mock-1.11.0, xdist-3.3.1, capture-warnings-0.0.4, icdiff-0.8
asyncio: mode=strict
setup timeout: 0.0s, execution timeout: 0.0s, teardown timeout: 0.0s
collected 1 item                                                                                                                                                                                                                    

tests/providers/amazon/aws/log/test_cloudwatch_task_handler.py::TestCloudwatchTaskHandler::test_write_json_logs[not-set] FAILED                                                                                               [100%]

============================================================================================================= FAILURES ==============================================================================================================
______________________________________________________________________________________ TestCloudwatchTaskHandler.test_write_json_logs[not-set] ______________________________________________________________________________________

self = <tests.providers.amazon.aws.log.test_cloudwatch_task_handler.TestCloudwatchTaskHandler object at 0x116b632e0>, mock_get_log_events = <MagicMock name='get_log_events' id='4843681872'>, conf_json_serialize = None
expected_serialized_output = '{"datetime": "2023-01-01T00:00:00+00:00", "customObject": null}'

    @pytest.mark.parametrize(
        "conf_json_serialize, expected_serialized_output",
        [
            pytest.param(
                "airflow.providers.amazon.aws.log.cloudwatch_task_handler.json_serialize_legacy",
                '{"datetime": "2023-01-01T00:00:00+00:00", "customObject": null}',
                id="json-serialize-legacy",
            ),
            pytest.param(
                "airflow.providers.amazon.aws.log.cloudwatch_task_handler.json_serialize",
                '{"datetime": "2023-01-01T00:00:00+00:00", "customObject": "SomeCustomSerialization(...)"}',
                id="json-serialize",
            ),
            pytest.param(
                None, '{"datetime": "2023-01-01T00:00:00+00:00", "customObject": null}', id="not-set"
            ),
        ],
    )
    @mock.patch.object(AwsLogsHook, "get_log_events")
    def test_write_json_logs(self, mock_get_log_events, conf_json_serialize, expected_serialized_output):
        class ToSerialize:
            def __init__(self):
                pass
    
            def __repr__(self):
                return "SomeCustomSerialization(...)"
    
        with conf_vars({("aws", "cloudwatch_task_handler_json_serializer"): conf_json_serialize}):
            handler = self.cloudwatch_task_handler
            handler.set_context(self.ti)
            message = logging.LogRecord(
                name="test_log_record",
                level=logging.DEBUG,
                pathname="fake.path",
                lineno=42,
                args=None,
                exc_info=None,
                msg={
                    "datetime": datetime(2023, 1, 1),
                    "customObject": ToSerialize(),
                },
            )
            with mock.patch("watchtower.threading.Thread"), mock.patch("watchtower.queue.Queue") as mq:
                mock_queue = Mock()
                mq.return_value = mock_queue
                handler.handle(message)
>               mock_queue.put.assert_called_once_with(
                    {"message": expected_serialized_output, "timestamp": ANY}
                )

tests/providers/amazon/aws/log/test_cloudwatch_task_handler.py:220: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../../.pyenv/versions/3.9.10/lib/python3.9/contextlib.py:126: in __exit__
    next(self.gen)
tests/test_utils/config.py:52: in conf_vars
    conf.set(section, key, value)
airflow/configuration.py:1304: in set
    super().set(section, option, value)
../../../.pyenv/versions/3.9.10/lib/python3.9/configparser.py:1204: in set
    super().set(section, option, value)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <airflow.configuration.AirflowConfigParser object at 0x110067f10>, section = 'aws', option = 'cloudwatch_task_handler_json_serializer'
value = 'airflow.providers.amazon.aws.log.cloudwatch_task_handler.json_serialize_legacy'

    def set(self, section, option, value=None):
        """Set an option."""
        if value:
            value = self._interpolation.before_set(self, section, option,
                                                   value)
        if not section or section == self.default_section:
            sectdict = self._defaults
        else:
            try:
                sectdict = self._sections[section]
            except KeyError:
>               raise NoSectionError(section) from None
E               configparser.NoSectionError: No section: 'aws'

It might not reproduced in CI, because some other tests adds some side effects ¯\_(ツ)_/¯ as result this and maybe others tests completed succesefully


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@Taragolis Taragolis added changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge labels Feb 15, 2024
@Taragolis Taragolis merged commit b840c14 into apache:main Feb 15, 2024
84 checks passed
@Taragolis Taragolis deleted the side-effects-conf-vars branch February 15, 2024 18:20
sunank200 pushed a commit to astronomer/airflow that referenced this pull request Feb 21, 2024
abhishekbhakat pushed a commit to abhishekbhakat/my_airflow that referenced this pull request Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants