def test_get_account(self, mock_paginate: Mock, mock_http_run: Mocj, conn_id,
account_id):
Might fix that? IDEs in general do not cope well with purest tests, and are
missing context on what most of the variables are, be it parameterised values
or fixture values, so this isn’t a problem that is unique to mocks.
> On 7 Jul 2025, at 12:47, Kyungjun Lee <kyungjunlee...@gmail.com> wrote:
>
> I'd like to follow up on our previous discussion about pytest-native vs
> unittest-style assertions.
>
> While working on the following test case:
>
> ```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"
>
> _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()
>
> ```
>
> My IDE shows a warning:
> Cannot find reference 'assert_called_once_with' in 'function'.
>
> This happens because the mock object is not properly recognized as a mock
> instance at type-checking time, which might confuse some contributors,
> especially new ones relying on type hints or static analysis tools.
>
> I realized that this aspect of mock handling might be missing from our
> previous discussions. I wanted to bring it up as part of the broader
> conversation about test styles—particularly how we balance IDE/tooling
> support with readability and style consistency.
>
> Curious to hear your thoughts on this!
>
> @ash @potiuk
>
> 2025년 7월 6일 (일) 오후 8:09, Kyungjun Lee <kyungjunlee...@gmail.com>님이 작성:
>
>> Thank you @Potiuk for pointing out the intent behind the “one assert per
>> test” principle, and @ash for highlighting how using dedicated mock assert
>> functions can make the code easier to read and understand. These were
>> perspectives I hadn’t fully considered, and I really appreciate you sharing
>> them.
>>
>> Thanks to your input, I was able to read more materials and broaden my
>> thinking on the topic. That said, my original focus was more on the idea
>> that sticking to plain assert statements lowers the entry barrier for new
>> contributors—because they don’t have to learn multiple assertion styles.
>>
>> Still, as @Potiuk mentioned, I’ll put more thought into making the testing
>> guidelines clearer and more concrete—especially since that helps AI-based
>> tools as well 😄
>>
>> For reference, here’s one of the articles I read:
>> https://medium.com/@kentbeck_7670/test-desiderata-94150638a4b3
>>
>> Thank you to everyone who took part in this discussion.
>>
>> 2025년 7월 6일 (일) 오후 3:42, Ash Berlin-Taylor <a...@apache.org>님이 작성:
>>
>>> I personally find the dedicated functions way easier to read the intent
>>> behind, it’s one function/statement vs 3. More so when you don’t care about
>>> the order of calls, just that something was called (where to do this
>>> manually we’d need to reimplement the helper function)
>>>
>>> Additionally pytest already rewrites those to have nicer error messages,
>>> but the dedicated mock assert finding are much easier to read the code and
>>> understand the intent to me, so I’i am for staying with the dedicated
>>> assert functions
>>>
>>> -ash
>>>
>>>> On 5 Jul 2025, at 13:30, Kyungjun Lee <kyungjunlee...@gmail.com> wrote:
>>>>
>>>> I’ve learned a lot of things I didn’t know before.
>>>> Thank you so much for all your help — I really appreciate it.
>>>> I’ll get started on this right away!
>>>>
>>>> 2025년 7월 5일 (토) 오후 9:18, Jarek Potiuk <ja...@potiuk.com>님이 작성:
>>>>
>>>>>> Haha ... I'm curious — which part sounded like ChatGPT style?
>>>>> English isn't my first language, so I did get a little help from it.
>>>>>
>>>>> That whole paragraph :) .
>>>>>
>>>>>>> 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.
>>>>>
>>>>> "Sure! Since you asked ..." sounds like an AI bot.
>>>>>
>>>>>> That got me thinking — what do you think about adding a guideline
>>>>> document
>>>>> for writing tests, similar to how we have a coding style guide?
>>>>>
>>>>> Absolutely - we already have some "seed' of it "Writing tests"
>>> chapter in
>>>>> contributing guideline, but we could definitely make it more detailed.
>>>>>
>>>>>
>>>>>
>>> https://github.com/apache/airflow/blob/main/contributing-docs/testing/unit_tests.rst#writing-unit-tests
>>>>>
>>>>> And - speaking of AI - this is becoming more and more important to
>>> describe
>>>>> any common rules we have and context - so that using Agentic AI yields
>>>>> better results. Kaxil already added
>>>>> https://github.com/apache/airflow/blob/main/AGENTS.md which describes
>>>>> context for coding agents lile Codex - and we could improve it and link
>>>>> more docs from our repo if they get more of our agreed "conventions" -
>>> then
>>>>> Agents would get it as context and their generated code would be
>>> consistent
>>>>> with what we describe there.
>>>>>
>>>>> In a way - I think having a good documentation on processes, tools and
>>>>> conventions was always something I've been after, but with the Agentic
>>>>> workflows it might significantly boost the quality of generated code
>>> if we
>>>>> have more of those conventions and guidelines described.
>>>>>
>>>>> So .... ABSOLUTELY ... the more we describe in there, the better. And
>>> we
>>>>> have no more excuse that "anyhow no-one reads it" - because the coding
>>>>> agents WILL be reading it and acting accordingly. So I think this is a
>>> very
>>>>> good investment to make.
>>>>>
>>>>> J.
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>
>>>>>> On Sat, Jul 5, 2025 at 2:07 PM Kyungjun Lee <kyungjunlee...@gmail.com
>>>>
>>>>>> wrote:
>>>>>>
>>>>>> Haha ... I'm curious — which part sounded like ChatGPT style?
>>>>>> English isn't my first language, so I did get a little help from it.
>>>>>>
>>>>>> But thank you — I actually learned something new from your comment!
>>>>>> That got me thinking — what do you think about adding a guideline
>>>>> document
>>>>>> for writing tests, similar to how we have a coding style guide? It
>>> might
>>>>>> help ensure consistency across the Airflow codebase when it comes to
>>>>>> testing styles as well.
>>>>>>
>>>>>> 2025년 7월 5일 (토) 오후 8:52, Jarek Potiuk <ja...@potiuk.com>님이 작성:
>>>>>>
>>>>>>> But of course - i'd love to hear what others think - it's not a "very
>>>>>>> strong" opinion.
>>>>>>>
>>>>>>> On Sat, Jul 5, 2025 at 1:48 PM Jarek Potiuk <ja...@potiuk.com>
>>> wrote:
>>>>>>>
>>>>>>>> Cool. That's what I wanted to see.
>>>>>>>>
>>>>>>>> By the way - not that there's anything wrong - but was the answer
>>>>>> written
>>>>>>>> by AI initially ? The first paragraph looks suspiciously like Chat
>>>>> GPT
>>>>>>>> answer :D ?
>>>>>>>>
>>>>>>>> Comment from my side: I personally prefer the original style. It's
>>>>> more
>>>>>>>> concise and it is easier to read - you see as if the call was
>>>>> actually
>>>>>>>> written down. Also this is quite a bit too many assertions in the
>>>>>> second
>>>>>>>> case and it takes a lot of mental effort to understand what actually
>>>>> is
>>>>>>>> being asserted.
>>>>>>>>
>>>>>>>> There is a "school" of writing unit tests that every test should
>>> have
>>>>>> ONE
>>>>>>>> assertion only. Always. I think it is a bit extreme, and I do not
>>>>>> follow
>>>>>>> it
>>>>>>>> myself but I think it is also a kind of good direction to have ->
>>> the
>>>>>>> fewer
>>>>>>>> assertions you have in your test, the better (I think).
>>>>>>>>
>>>>>>>> I think tests should be mostly optimized for easiness of reading and
>>>>>>>> understanding what is being tested - and it's just not that easy in
>>>>> the
>>>>>>>> second case.
>>>>>>>>
>>>>>>>> J.
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> On Sat, Jul 5, 2025 at 1:39 PM Kyungjun Lee <
>>>>> kyungjunlee...@gmail.com>
>>>>>>>> wrote:
>>>>>>>>
>>>>>>>>> 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
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
>>> For additional commands, e-mail: dev-h...@airflow.apache.org
>>>
>>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
For additional commands, e-mail: dev-h...@airflow.apache.org
