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

Integration tests are separated into separate command and CI job #28207

Merged
merged 1 commit into from
Dec 8, 2022

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Dec 7, 2022

Integration tests so far were a separate test type among the
unit tests, however we have to start them differently.

This PR introduces new command in Breeze:

breeze testing integration-tests

The --integration option has been removed from the regular
unit tests, and now it is used to the integration-tests command.

The integration-tests command has no parallel option.


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

@potiuk potiuk changed the title Sepearate integration tests Separate integration tests Dec 7, 2022
@potiuk potiuk changed the title Separate integration tests Integration tests are separated into separate command and CI job Dec 7, 2022
@potiuk potiuk force-pushed the sepearate-integration-tests branch 4 times, most recently from 54fe101 to c7a326f Compare December 7, 2022 23:30
@potiuk potiuk marked this pull request as ready for review December 7, 2022 23:30
@potiuk potiuk force-pushed the sepearate-integration-tests branch from c7a326f to 5fa0c48 Compare December 7, 2022 23:36
@potiuk potiuk added the use public runners Makes sure that Public runners are used even if commiters creates the PR (useful for testing) label Dec 7, 2022
@potiuk potiuk force-pushed the sepearate-integration-tests branch 3 times, most recently from b6553bc to d0b65b7 Compare December 8, 2022 00:13
Integration tests so far were a separate test type among the
unit tests, however we have to start them differently.

This PR introduces new command in Breeze:

breeze testing integration-tests

The `--integration` option has been removed from the regular
unit tests, and now it is used to the integration-tests command.

The integration-tests command has no parallel option.
@potiuk potiuk force-pushed the sepearate-integration-tests branch from d0b65b7 to 7a1c832 Compare December 8, 2022 00:59
@potiuk
Copy link
Member Author

potiuk commented Dec 8, 2022

This one is based on #28209 (so #28209 should be merged first) - and it should deal with the memory issues we had in public runners (it should also speed up our tests):

  1. Unit tests now do not have "Integration" as test type - so regular "matrix" tests do not run integration tests. Much less memory used for those jobs in general.

  2. We have a new command in breeze breeze testing integration-tests that allows to run integration tests very easyily - for example breeze testing integration-tests --integration mongo will run mongo db tests, breeze testing integration-tests --integration all will run all integration tests

  3. I have removed the old openldap integration (it turned out it is not used any more)

  4. We are using the breeze command in CI in a separate job to run integration tests

  5. In Self-hosted runners we run all integration tests for Postgres at once with all integrations enabled (because memory!)

  6. In Self-hosted runners we also additionaly run all tests for MySQL

  7. In Public runners we run the integration tests sequentially (cassandra, mongo, pinot, celery, (trino + kerberos) and we run breeze stop between each integration test type - this way we only run one/two docker-compose integrations at a time and we stop/reclaim memory after each integration test type

@potiuk
Copy link
Member Author

potiuk commented Dec 8, 2022

Screenshot 2022-12-08 at 02 03 44

@potiuk
Copy link
Member Author

potiuk commented Dec 8, 2022

Screenshot 2022-12-08 at 02 04 20

Copy link
Member

@uranusjr uranusjr left a comment

Choose a reason for hiding this comment

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

Makes sense. (I kind of not like the verbosity of breeze testing integration-tests --integration ... but I guess that can work.

@potiuk
Copy link
Member Author

potiuk commented Dec 8, 2022

Makes sense. (I kind of not like the verbosity of breeze testing integration-tests --integration ... but I guess that can work.

That was the most coherent one witht the rest of the system: --integration is also used in regular shell command - if you want to enter breeze and start selected integration with it.

@potiuk potiuk merged commit b37452e into apache:main Dec 8, 2022
@potiuk potiuk deleted the sepearate-integration-tests branch December 8, 2022 08:43
ephraimbuddy pushed a commit that referenced this pull request Jan 13, 2023
)

Integration tests so far were a separate test type among the
unit tests, however we have to start them differently.

This PR introduces new command in Breeze:

breeze testing integration-tests

The `--integration` option has been removed from the regular
unit tests, and now it is used to the integration-tests command.

The integration-tests command has no parallel option.

(cherry picked from commit b37452e)
ephraimbuddy pushed a commit that referenced this pull request Jan 14, 2023
)

Integration tests so far were a separate test type among the
unit tests, however we have to start them differently.

This PR introduces new command in Breeze:

breeze testing integration-tests

The `--integration` option has been removed from the regular
unit tests, and now it is used to the integration-tests command.

The integration-tests command has no parallel option.

(cherry picked from commit b37452e)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dev-tools use public runners Makes sure that Public runners are used even if commiters creates the PR (useful for testing)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants