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