Skip to content

Add validation tests for GenAIGenerateContentOperator#61262

Closed
KL-2300032590 wants to merge 6 commits into
apache:mainfrom
KL-2300032590:test-gcp-genai-operator
Closed

Add validation tests for GenAIGenerateContentOperator#61262
KL-2300032590 wants to merge 6 commits into
apache:mainfrom
KL-2300032590:test-gcp-genai-operator

Conversation

@KL-2300032590
Copy link
Copy Markdown

This PR adds validation-focused unit tests for GenAIGenerateContentOperator
to ensure the operator fails fast when required parameters are missing.

This work contributes to #61261.

Notes:

Copy link
Copy Markdown
Contributor

@shahar1 shahar1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good start!
Personally I don't find the tests you've implemented too practical, as ususally the type hints help us with figuring out such discrepancies (you could retain the tests though, they're not harmful 🙂).
Could you please try to test the execute method for the case where all required parameters are given?
Try to look at unit tests of other providers and learn how mocking works.
Please let me know if you need some direction.

Comment thread providers/google/tests/unit/google/cloud/operators/test_gen_ai.py Outdated
Comment thread providers/postgres/tests/unit/postgres/hooks/test_postgres.py
@shahar1 shahar1 self-requested a review February 1, 2026 17:47
Copy link
Copy Markdown
Contributor

@shahar1 shahar1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CI seems to fail now :(
Please let me know if you need some guidance.

@KL-2300032590
Copy link
Copy Markdown
Author

KL-2300032590 commented Feb 2, 2026

CI seems to fail now :( Please let me know if you need some guidance.

@shahar1,
thanks for the review and the offer to help
CI is still failing across multiple provider/distribution jobs. Before making further changes, could you please point me to which specific assertion or behavior in the GenAI operator tests is incompatible with current expectations, or which check you’d recommend I focus on first?

@shahar1 shahar1 self-requested a review February 2, 2026 09:32
@shahar1
Copy link
Copy Markdown
Contributor

shahar1 commented Feb 2, 2026

CI seems to fail now :( Please let me know if you need some guidance.
@shahar1,
thanks for the review and the offer to help
CI is still failing across multiple provider/distribution jobs. Before making further changes, could you please point me to which specific assertion or behavior in the GenAI operator tests is incompatible with current expectations, or which check you’d recommend I focus on first?

CI seems to fail now :( Please let me know if you need some guidance.

@shahar1, thanks for the review and the offer to help CI is still failing across multiple provider/distribution jobs. Before making further changes, could you please point me to which specific assertion or behavior in the GenAI operator tests is incompatible with current expectations, or which check you’d recommend I focus on first?

Hey, I'd like to let you know that I've made a mistake and the tests which I thought were missing, actually exist in this file:
https://github.com/apache/airflow/blob/main/providers/google/tests/unit/google/cloud/operators/test_gen_ai.py#L142

I also overlooked that the test file you proposed was in the wrong path (it should be in providers/google/test/unit/google) which partially caused this confusion when reviewing this PR.

However, you may still use this PR to try and test the exceptions raised in the current source file and currently not tested, for example:

Try to look at the current test file, as well as others - and then you could try to utilize this PR for working on the exceptions.
My sincere apologies for the mess, I'll convert this PR to draft for now. If you plan to reuse it for testing the exceptions, please comment.

@shahar1 shahar1 marked this pull request as draft February 2, 2026 09:40
@KL-2300032590
Copy link
Copy Markdown
Author

CI seems to fail now :( Please let me know if you need some guidance.
@shahar1,
thanks for the review and the offer to help
CI is still failing across multiple provider/distribution jobs. Before making further changes, could you please point me to which specific assertion or behavior in the GenAI operator tests is incompatible with current expectations, or which check you’d recommend I focus on first?

CI seems to fail now :( Please let me know if you need some guidance.

@shahar1, thanks for the review and the offer to help CI is still failing across multiple provider/distribution jobs. Before making further changes, could you please point me to which specific assertion or behavior in the GenAI operator tests is incompatible with current expectations, or which check you’d recommend I focus on first?

Hey, I'd like to let you know that I've made a mistake and the tests which I thought were missing, actually exist in this file: https://github.com/apache/airflow/blob/main/providers/google/tests/unit/google/cloud/operators/test_gen_ai.py#L142

I also overlooked that the test file you proposed was in the wrong path (it should be in providers/google/test/unit/google) which partially caused this confusion when reviewing this PR.

However, you may still use this PR to try and test the exceptions raised in the current source file and currently not tested, for example:

Try to look at the current test file, as well as others - and then you could try to utilize this PR for working on the exceptions. My sincere apologies for the mess, I'll convert this PR to draft for now. If you plan to reuse it for testing the exceptions, please comment.

Hi @shahar1,

Thanks for the clarification — that makes sense now. I understand that the validation tests already exist and that the confusion was partly due to the test path.

I’m fine with repurposing this PR to focus only on currently untested exception paths in the GenAI operators (e.g. explicit AirflowExceptions).

Appreciate the guidance.

@github-actions
Copy link
Copy Markdown
Contributor

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions.

@github-actions github-actions Bot added the stale Stale PRs per the .github/workflows/stale.yml policy file label Mar 20, 2026
@github-actions github-actions Bot closed this Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:providers provider:google Google (including GCP) related issues stale Stale PRs per the .github/workflows/stale.yml policy file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants