Sure! Since you asked for how the test *should* be written, I took the opportunity to clean it up using a more pytest-native style while also fixing the mock order issue.
Here’s the updated test: ```python @pytest.mark.parametrize( argnames="conn_id, account_id", argvalues=[(ACCOUNT_ID_CONN, None), (NO_ACCOUNT_ID_CONN, ACCOUNT_ID)], ids=["default_account", "explicit_account"], ) @patch.object(DbtCloudHook, "run") @patch.object(DbtCloudHook, "_paginate") def test_get_account(self, mock_paginate, mock_http_run, conn_id, account_id): hook = DbtCloudHook(conn_id) hook.get_account(account_id=account_id) assert hook.method == "GET" expected_account_id = account_id or DEFAULT_ACCOUNT_ID assert mock_http_run.call_count == 1 assert mock_http_run.call_args.args == () assert mock_http_run.call_args.kwargs == { "endpoint": f"api/v2/accounts/{expected_account_id}/", "data": None, "extra_options": None, } assert mock_paginate.call_count == 0 ``` Why I chose this style: - *Mock verification using assert*: Instead of mock.assert_called_once_with(...), I used call_count and call_args. This approach aligns better with pytest’s idioms and produces cleaner, more readable error messages when assertions fail. - *Explicit verification*: Using call_args.args and call_args.kwargs makes the test behavior very explicit, which helps with debugging and understanding the exact calls made. - *Decorator order matching argument order*: As @patch decorators apply from the bottom up, the argument order has been corrected to match ( mock_paginate first, then mock_http_run). Let me know if you'd like to follow a slightly different convention — happy to adjust! I was lucky to have the chance to explain this while fixing a related bug. You can refer to the changes in this PR: https://github.com/apache/airflow/pull/52905 2025년 7월 5일 (토) 오후 8:09, Jarek Potiuk <ja...@potiuk.com>님이 작성: > Just post how you think the test should be written :) > > On Sat, Jul 5, 2025 at 1:08 PM Jarek Potiuk <ja...@potiuk.com> wrote: > > > I already mentioned it in slack - but how would you propose to rewrite > the > > "mixed" case to be more consistent ? > > > > On Sat, Jul 5, 2025 at 12:47 PM Kyungjun Lee <kyungjunlee...@gmail.com> > > wrote: > > > >> Hi all, > >> > >> While reviewing and contributing to Airflow tests, I’ve noticed an > >> inconsistency in how assertions are written. Some tests use > >> `unittest`-style assertions like `mock.assert_called_once_with`, while > >> others use plain `assert` statements in the `pytest` style. > >> > >> Here's an example of a test using the mixed style: > >> > >> ```python > >> @pytest.mark.parametrize( > >> argnames="conn_id, account_id", > >> argvalues=[(ACCOUNT_ID_CONN, None), (NO_ACCOUNT_ID_CONN, > ACCOUNT_ID)], > >> ids=["default_account", "explicit_account"], > >> ) > >> @patch.object(DbtCloudHook, "run") > >> @patch.object(DbtCloudHook, "_paginate") > >> def test_get_account(self, mock_http_run, mock_paginate, conn_id, > >> account_id): > >> hook = DbtCloudHook(conn_id) > >> hook.get_account(account_id=account_id) > >> > >> assert hook.method == "GET" > >> > >> _account_id = account_id or DEFAULT_ACCOUNT_ID > >> hook.run.assert_called_once_with( > >> endpoint=f"api/v2/accounts/{_account_id}/", data=None, > >> extra_options=None > >> ) > >> hook._paginate.assert_not_called() > >> ``` > >> > >> In IDEs and type-checkers (like PyCharm or MyPy), this sometimes causes > >> weak warnings > >> > >> ``` > >> > >> Cannot find reference 'assert_called_once_with' in 'function' > >> > >> ``` > >> > >> This could confuse newcomers or contributors unfamiliar with mocking > >> behavior or type limitations in dynamic typing. > >> > >> To improve clarity and accessibility for contributors—especially those > new > >> to the project—I’d like to propose *moving toward consistent use of > plain > >> assert statements* for test validations wherever possible. > >> > >> *Proposed Benefits*: > >> > >> - > >> > >> Easier onboarding for first-time contributors > >> - > >> > >> Better IDE support and fewer confusing warnings > >> - > >> > >> More consistent and readable test style across the project > >> > >> I'd love to hear your thoughts on whether this direction makes sense for > >> the project. If agreed, I’d also be happy to help align existing tests > >> gradually. > >> > >> Thanks! > >> Kyungjun Lee > >> > > >