It **looks** like we are close to consensus on that one. I think a good
idea would be to create an outline proposal summarizing what we are going
to have finally, ask people for comments if anything has anything against
it, and if we do not see any "no' - create a PR with more details in the
guideline an calling officially for "[LAZY CONSENSUS]" on it (and in
the meantime proceed with implementation :D.

J

On Fri, Jul 11, 2025 at 11:36 AM Kyungjun Lee <kyungjunlee...@gmail.com>
wrote:

> Should we start by drafting the document first, or should we ask the
> community through a vote whether it’s okay to create the guideline?
>
>
>
>
>
> 2025년 7월 11일 (금) 오후 6:31, Kyungjun Lee <kyungjunlee...@gmail.com>님이 작성:
>
> > I want to lead this discussion!!!
> >
> > 2025년 7월 11일 (금) 오후 6:18, Jarek Potiuk <ja...@potiuk.com>님이 작성:
> >
> >> We just need a volunteer (or few) to drive it :D
> >>
> >> On Fri, Jul 11, 2025 at 11:15 AM Jarek Potiuk <ja...@potiuk.com> wrote:
> >>
> >> > Yeah. I like it and the discussion :)  - it's so cool to see so many
> >> > people taking part and leading the discussion. The Airflow community
> is
> >> > awesome.
> >> >
> >> > I think it's nice with pytest-mock + the "pytest-first" patterns. I
> >> think
> >> > it would be cool to indeed propose a "guideline" first and then
> possibly
> >> > even rewrite things and add some guardrails to flag non-pytest usages
> >> > automatically (pre-commit) - that is always the best to not only
> >> "agree" on
> >> > something but automate keeping that in-check.
> >> >
> >> > We could likely - again - use AI and run a small mini-project to
> convert
> >> > all our tests. I think if we write detailed-enough guideline, we can
> >> > literally prompt AI "rewrite the test code here following the
> guidelines
> >> > here".
> >> >
> >> > So what I see as an "outcome of a discussion" is:
> >> > * to collaboratively work on the guidelines
> >> > * make them both human and AI digestible
> >> > * add guardrails to not add old style in new files and to gradually
> mark
> >> > parts of the code as "done" when done
> >> > * run the "mini-project" to apply it consistently (and maybe as a
> >> > result iterate and correct and improve the guidelines)
> >> >
> >> > J.
> >> >
> >> >
> >> > On Fri, Jul 11, 2025 at 7:57 AM Amogh Desai <amoghdesai....@gmail.com
> >
> >> > wrote:
> >> >
> >> >> +1 to TP's proposal too.
> >> >>
> >> >> It's easy to read and also stands out better.
> >> >>
> >> >> We have a few places in the task-sdk tests where we also have done
> >> >> patterns
> >> >> like:
> >> >>
> >> >> assert not any(
> >> >>     x
> >> >>     == mock.call(
> >> >>         msg=GetXCom(
> >> >>             key="key",
> >> >>             dag_id="test_dag",
> >> >>             run_id="test_run",
> >> >>             task_id="pull_task",
> >> >>             map_index=-1,
> >> >>         ),
> >> >>     )
> >> >>     for x in mock_supervisor_comms.send.call_args_list
> >> >> )
> >> >>
> >> >>
> >> >> call_args_list is particularly useful in scenarios when you want to
> >> >> validate presence / absence of a call.
> >> >>
> >> >> Thanks & Regards,
> >> >> Amogh Desai
> >> >>
> >> >>
> >> >> On Thu, Jul 10, 2025 at 8:39 PM Kyungjun Lee <
> kyungjunlee...@gmail.com
> >> >
> >> >> wrote:
> >> >>
> >> >> > 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
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >> >
> >>
> >
>

Reply via email to