Add prek hook to ban caplog usage in test files#65421
Add prek hook to ban caplog usage in test files#65421amoghrajesh wants to merge 1 commit intoapache:mainfrom
Conversation
|
Nice improvement, I agree that we should to add the prek hook to prevent more caplog usage in the first place. |
bugraoz93
left a comment
There was a problem hiding this comment.
Nice, thanks for including these, Amogh! I think we were checking in the root beforehand
da67f8a to
282aca5
Compare
|
Rebased to run with "full tests" |
|
We should use the label for this now as it fails for all and maybe for accepting some if needed |
|
Since we call directly pygrep maybe we need to add skip option to prek but I don't know how or even possible |
jscheffl
left a comment
There was a problem hiding this comment.
Cool!
But probably you need to add an exclusion list for all cases still in the repo or are we (still) clean?
Oh, just see the list of static checks, seems like a call for helping hands is needed! |
|
Oh haha, seems I need to do that. Let me take a stab at that |
Was generative AI tooling used to co-author this PR?
Noticed recently in #65232 that caplog was used.
Wanted to add a prek hook to now allow this as per our coding standards: https://github.com/apache/airflow/blob/main/AGENTS.md#testing-standards.
Intent is to enforce the testing standard that tests should check logic rather than log output.
I am proposing to solve this by adding a
pygrephook (check-no-caplog-in-tests) toairflow-core,providers, andtask-sdkpre commit config. The hook matches\bcaplog\bin test files and fails if any new usage is introduced.Existing usages (35 core, 89 provider, 6 task-sdk files) are unaffected — prek only runs on files changed in a given PR, so cleanup can happen incrementally.
{pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.