Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move OpenAPI tests to breeze container #44326

Merged
merged 15 commits into from
Nov 27, 2024

Conversation

gopidesupavan
Copy link
Member

closes: #44020

Running Open API client tests directly in the runner environment is fragile due to the need for manual installation of Airflow and providers, which can cause dependency conflicts and installation errors. Breeze offers a consistent, containerized setup.

Moving openapi tests run inside breeze container. Breeze uses a pre-configured container image, avoiding the need for manual installation of Airflow and providers in the runner


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

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

THat all looks very cool! Let's see if CI turns green... then then LGTM in my view!

@gopidesupavan gopidesupavan added the full tests needed We need to run full set of tests for this PR to merge label Nov 24, 2024
@gopidesupavan gopidesupavan reopened this Nov 24, 2024
@gopidesupavan
Copy link
Member Author

Full tests not triggering , might be because of changes to workflows? @potiuk does this change need to come from the apache repo?

@gopidesupavan
Copy link
Member Author

image

@gopidesupavan
Copy link
Member Author

ah it seems this one:

image

@gopidesupavan
Copy link
Member Author

ah it seems this one:

image

This looks like github actions have hard limit.

@gopidesupavan
Copy link
Member Author

Alright one test failing but, its not related to this i believe. i will check that separate..

@gopidesupavan
Copy link
Member Author

Full tests not triggering , might be because of changes to workflows? @potiuk does this change need to come from the apache repo?

This is fixed after removing openapi tests workflow :)

@potiuk
Copy link
Member

potiuk commented Nov 26, 2024

Added some comments - it looks fantastic in general, but I found a few places where things might be improved. We could also use the opportunity to rename those tests to be "python-api-client-tests" across the board.

@gopidesupavan
Copy link
Member Author

Added some comments - it looks fantastic in general, but I found a few places where things might be improved. We could also use the opportunity to rename those tests to be "python-api-client-tests" across the board.

cool, thanks those really great improvement suggestions let me have a updates on those :)

@potiuk
Copy link
Member

potiuk commented Nov 26, 2024

BTW. I really like how this tests openapi-tests (or whatever we name it eventually) is running all the tests of the client as well that gets generated :)

image

Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Nice! Just the last two changes !

.github/workflows/ci.yml Outdated Show resolved Hide resolved
@gopidesupavan gopidesupavan merged commit 25432dc into apache:main Nov 27, 2024
95 checks passed
@gopidesupavan gopidesupavan deleted the move-open-api-tests branch November 27, 2024 13:15
@potiuk
Copy link
Member

potiuk commented Nov 27, 2024

Cool!

ArshiaZr pushed a commit to ArshiaZr/airflow that referenced this pull request Nov 27, 2024
* move openapi tests to breeze container

* remove space

* move openapi tests inside special tests workflow

* merge openapi tests into special tests workflow

* use postgres backend for openapi tests

* ignore tmp folder tests discovery in openapi tests

* adding retries to python client tests connection

* adding retries to python client tests connection

* export auth backend configs

* use connectivity check to verify webserver started or not

* rename is-special-tests-require to special-tests-required

* rename openapi-tests to python-api-client-tests

* rename static-checks-mypy-docs.yml to ci-image-checks.yml

* rename static-checks-mypy-docs.yml to ci-image-checks.yml

* remove special-tests-required param
prabhusneha pushed a commit to astronomer/airflow that referenced this pull request Nov 28, 2024
* move openapi tests to breeze container

* remove space

* move openapi tests inside special tests workflow

* merge openapi tests into special tests workflow

* use postgres backend for openapi tests

* ignore tmp folder tests discovery in openapi tests

* adding retries to python client tests connection

* adding retries to python client tests connection

* export auth backend configs

* use connectivity check to verify webserver started or not

* rename is-special-tests-require to special-tests-required

* rename openapi-tests to python-api-client-tests

* rename static-checks-mypy-docs.yml to ci-image-checks.yml

* rename static-checks-mypy-docs.yml to ci-image-checks.yml

* remove special-tests-required param
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:API Airflow's REST/HTTP API area:dev-tools full tests needed We need to run full set of tests for this PR to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Test OpenAPI client tests
3 participants