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