Thank you all — I really appreciate how many people have joined the
discussion.
I also like the approach that Tzu-ping Chung suggested. This really isn’t
an easy topic. At first, I thought it would be best to use only plain assert
statements, but after reading through the different perspectives here, I’ve
come to realize that overusing assert can also be problematic. It’s been a
great reminder that we should be intentional about what we choose to assert
— making sure we’re testing what truly matters.
I’ll also follow up soon with an example snippet and a brief testing guide
to help clarify the discussion.
2025년 7월 10일 (목) 오후 11:49, Tzu-ping Chung <t...@astronomer.io.invalid>님이 작성:
> Does pytest-mock have an equivalent for call()? I agree for mocking in
> general we should consider replacing plain decorators and context managers
> with the mocker fixture. This probably deserves its own discussion thread.
>
> --
> Sent from my iPhone
>
> > On 10 Jul 2025, at 14:37, Dev-iL <gid....@gmail.com> wrote:
> >
> > One tiny comment regarding TP's suggestion - IMHO it's better to avoid
> `unittest.mock` in favor of the equivalent `mocker` fixture provided by
> `pytest-mock`.
> >
> > On 2025/07/10 06:30:22 Tzu-ping Chung wrote:
> > > Personally I dislike things like assert_called_once_with etc. since
> they are easy to miss when you scan a test to see what they are trying to
> check. An 'assert' keyword stands out (it’s always the first word in the
> line), especially with syntax highlighting.
> > >
> > > I do agree the proposed Pytest style is arguably less readable. I
> offer another syntax.
> > >
> > >
> > > from unittest.mock import call
> > >
> > > assert mock_http_run.mock_calls == [
> > > call(
> > > endpoint=f"api/v2/accounts/{expected_account_id}/",
> > > data=None,
> > > extra_options=None,
> > > )
> > > ]
> > > assert mock_paginate.mock_calls == []
> > >
> > >
> > > To me, this is on par with assert_called_once_with et al. in terms of
> readability, and arguably better for test authors since you don’t need to
> remember the various function names anymore, but only 'mock_calls' and the
> 'call' helper class.
> > >
> > > TP
> > >
> > >
> > > > On Jul 9, 2025, at 23:28, Ferruzzi, Dennis <fe...@amazon.com.INVALID>
> wrote:
> > > >
> > > > I'm a bit late to the party, and really only reiterating what has
> already been said, but of the two examples (original and your rewrite, I
> prefer the original. I think as a general rule, we tend to use the
> assert_called_once, etc with mocks butt he asserts with non-mocked
> variables.
> > > >
> > > > I am all for more documentation, but I'd have a slight preference
> towards keeping the existing structure.
> > > >
> > > >
> > > > - ferruzzi
> > > >
> > > >
> > > > ________________________________
> > > > From: Kyungjun Lee <ky...@gmail.com>
> > > > Sent: Tuesday, July 8, 2025 2:13 AM
> > > > To: dev@airflow.apache.org
> > > > Subject: RE: [EXT] [DISCUSS] Consistent test assertion style:
> pytest-native vs unittest-style
> > > >
> > > > CAUTION: This email originated from outside of the organization. Do
> not click links or open attachments unless you can confirm the sender and
> know the content is safe.
> > > >
> > > >
> > > >
> > > > AVERTISSEMENT: Ce courrier électronique provient d’un expéditeur
> externe. Ne cliquez sur aucun lien et n’ouvrez aucune pièce jointe si vous
> ne pouvez pas confirmer l’identité de l’expéditeur et si vous n’êtes pas
> certain que le contenu ne présente aucun risque.
> > > >
> > > >
> > > >
> > > > Thank you Ash and Amogh Desai for your insights and explanations.
> > > > The information you shared has been incredibly helpful and is
> contributing
> > > > a lot to my growth.
> > > >
> > > > 2025년 7월 8일 (화) 오후 2:54, Amogh Desai <am...@gmail.com>님이 작성:
> > > >
> > > >> Agreed, I personally also find the current way to be easier to read
> and in
> > > >> most
> > > >> cases we want to assert if "something" was called, irrespective of
> the
> > > >> order it was
> > > >> called in. So the dedicated function based way works well for me.
> > > >>
> > > >> If I want to test a order, I'd rather call parts of code that I
> want to
> > > >> test explicitly and assert
> > > >> on them.
> > > >>
> > > >>> 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.
> > > >>
> > > >> Regarding this, I'd say you can either cast mocks to their types as
> one
> > > >> way:
> > > >> `mock_http_run: MagicMock = mock_http_run` -- give or take, or use
> > > >> `autospec` to make the mock reflect the signature of the object?
> Check out:
> > > >> https://docs.python.org/3/library/unittest.mock.html#autospeccing
> > > >>
> > > >> Thanks & Regards,
> > > >> Amogh Desai
> > > >>
> > > >>
> > > >> On Mon, Jul 7, 2025 at 6:13 PM Ash Berlin-Taylor <as...@apache.org>
> wrote:
> > > >>
> > > >>> 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 <ky...@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 <ky...@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 <as...@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 <ky...@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 lu
> > [message truncated...]
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@airflow.apache.org
> For additional commands, e-mail: dev-h...@airflow.apache.org
>
>
