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