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

Improve testing harness to separate DB and non-DB tests #35160

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Oct 24, 2023

This PR marks DB tests as such and allows to split execution
of the tests in CI to run the DB tests with the various database
while the non-db tests - without the DB in a separate run.

In order to do that, the code to select which tests to run has been
moved from entrypoint_ci.sh bash to breeze's Python code, which is
generally much nicer to maintain and common for both "DB" and
"non-DB" tests.

This will have the nice side effect that it will be easier in the
future to manage different test types and contain some specific
flaky test types.

This change also adds possibility to isolate some of the test types
when parallel DB tests are run and adds new test type
PythonOperator carved out Operator type. This test is best run in
isolation becasue creating and destroing virtualenvs in Docker while
running in parallel to other tests is very slow for some reason and
leads to flaky tests.

Python operator tests are therefore separated out from Operators and
treated separately as isolated tests.

This will help not only with speed but also with stability of the
test suite.


^ 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
Copy link
Member Author

potiuk commented Oct 24, 2023

Devlist proposal coming based on that one.

@potiuk potiuk changed the title Add support for pure unit tests Add support for marking tests as DB tests Oct 24, 2023
@potiuk potiuk force-pushed the pure-unit-tests branch 20 times, most recently from 53ec866 to 58540c9 Compare October 26, 2023 13:19
@potiuk potiuk marked this pull request as ready for review October 26, 2023 13:25
This PR marks DB tests as such and allows to split execution
of the tests in CI to run the DB tests with the various database
while the non-db tests - without the DB in a separate run.

In order to do that, the code to select which tests to run has been
moved from `entrypoint_ci.sh` bash to breeze's Python code, which is
generally much nicer to maintain and common for both "DB" and
"non-DB" tests.

This will have the nice side effect that it will be easier in the
future to manage different test types and contain some specific
flaky test types.

This change also adds possibility to isolate some of the test types
when parallel DB tests are run and adds new test type
PythonOperator carved out Operator type. This test is best run in
isolation becasue creating and destroing virtualenvs in Docker while
running in parallel to other tests is very slow for some reason and
leads to flaky tests.

Python operator tests are therefore separated out from Operators and
treated separately as isolated tests.

This will help not only with speed but also with stability of the
test suite.
@potiuk potiuk merged commit a7e76ba into apache:main Oct 31, 2023
71 checks passed
@potiuk potiuk deleted the pure-unit-tests branch October 31, 2023 20:52
@potiuk
Copy link
Member Author

potiuk commented Oct 31, 2023

BOOM ,, MERGED

@potiuk potiuk added this to the Airflow 2.7.3 milestone Nov 1, 2023
@potiuk potiuk added the area:dev-env CI, pre-commit, pylint and other changes that do not change the behavior of the final code label Nov 1, 2023
@ephraimbuddy ephraimbuddy added the type:misc/internal Changelog: Misc changes that should appear in change log label Nov 1, 2023
ephraimbuddy pushed a commit that referenced this pull request Nov 1, 2023
This PR is separated out from #35160 in order to make the big
refactor and splitting out our test harness to DB and Non-DB
tests far more easy to review.

This change contains purely adding db_tests markers to the tests
that need them, and as such it requires very litte effort to
review. Once this one is merged hoever, the #35160 will become
way smaller in terms of number of files to review, which will
make it far easier to review using GitHub Interface.

(cherry picked from commit 1aa91a4)
ephraimbuddy pushed a commit that referenced this pull request Nov 1, 2023
This PR marks DB tests as such and allows to split execution
of the tests in CI to run the DB tests with the various database
while the non-db tests - without the DB in a separate run.

In order to do that, the code to select which tests to run has been
moved from `entrypoint_ci.sh` bash to breeze's Python code, which is
generally much nicer to maintain and common for both "DB" and
"non-DB" tests.

This will have the nice side effect that it will be easier in the
future to manage different test types and contain some specific
flaky test types.

This change also adds possibility to isolate some of the test types
when parallel DB tests are run and adds new test type
PythonOperator carved out Operator type. This test is best run in
isolation becasue creating and destroing virtualenvs in Docker while
running in parallel to other tests is very slow for some reason and
leads to flaky tests.

Python operator tests are therefore separated out from Operators and
treated separately as isolated tests.

This will help not only with speed but also with stability of the
test suite.

(cherry picked from commit a7e76ba)
potiuk added a commit that referenced this pull request Nov 4, 2023
Turns out that some of the layers in our PROD image got
invalidated because AIRFLOW_CONSTRAINTS_MODE used to build the
cache for PROD image is "constraints" by default, while building
images in "build-images" workflow for regular PRs and canary
build uses "constraints-source-providers". The former is fine as
default for PROD image (as oppose to CI image we build PROD image
from released PyPI packages by default) but the latter is "proper"
for the CI cache, because there, the image is built out of local
packages prepared from sources.

Turns out that the CONSTRAINT_MODE parameter had a profound impact
on caching - because it was set before the
"install_packages_from_branch_tip" step and - in fact - even
before "install database clients" step, which caused our cache to
only work for the "base OS dependencies" - installing database
clients and installing airflow from branch tip (which works great
for CI image) had always been done in PRs because the layers in
cache with constraints env invalidated all subsequent layers.

This had no big impact before when testing usually took much longer
time - but since the testing has been vastly improved in #35160, now
PROD image building continues running even after test complete and
becomes the next frontier of optimization.

This PR optimizes PROD image building in two ways:

* caching is prepared with "source_providers" constraint mode, same
  as regular build

* the AIRFLOW_CONSTRAINT_MODE and related arguments are moved after
  installing database clients, so that this parameter does not
  impact their caching.
potiuk added a commit that referenced this pull request Nov 4, 2023
Turns out that some of the layers in our PROD image got
invalidated because AIRFLOW_CONSTRAINTS_MODE used to build the
cache for PROD image is "constraints" by default, while building
images in "build-images" workflow for regular PRs and canary
build uses "constraints-source-providers". The former is fine as
default for PROD image (as oppose to CI image we build PROD image
from released PyPI packages by default) but the latter is "proper"
for the CI cache, because there, the image is built out of local
packages prepared from sources.

Turns out that the CONSTRAINT_MODE parameter had a profound impact
on caching - because it was set before the
"install_packages_from_branch_tip" step and - in fact - even
before "install database clients" step, which caused our cache to
only work for the "base OS dependencies" - installing database
clients and installing airflow from branch tip (which works great
for CI image) had always been done in PRs because the layers in
cache with constraints env invalidated all subsequent layers.

This had no big impact before when testing usually took much longer
time - but since the testing has been vastly improved in #35160, now
PROD image building continues running even after test complete and
becomes the next frontier of optimization.

This PR optimizes PROD image building in two ways:

* caching is prepared with "source_providers" constraint mode, same
  as regular build

* the AIRFLOW_CONSTRAINT_MODE and related arguments are moved after
  installing database clients, so that this parameter does not
  impact their caching.
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Nov 10, 2023
This PR marks DB tests as such and allows to split execution
of the tests in CI to run the DB tests with the various database
while the non-db tests - without the DB in a separate run.

In order to do that, the code to select which tests to run has been
moved from `entrypoint_ci.sh` bash to breeze's Python code, which is
generally much nicer to maintain and common for both "DB" and
"non-DB" tests.

This will have the nice side effect that it will be easier in the
future to manage different test types and contain some specific
flaky test types.

This change also adds possibility to isolate some of the test types
when parallel DB tests are run and adds new test type
PythonOperator carved out Operator type. This test is best run in
isolation becasue creating and destroing virtualenvs in Docker while
running in parallel to other tests is very slow for some reason and
leads to flaky tests.

Python operator tests are therefore separated out from Operators and
treated separately as isolated tests.

This will help not only with speed but also with stability of the
test suite.
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Nov 10, 2023
Turns out that some of the layers in our PROD image got
invalidated because AIRFLOW_CONSTRAINTS_MODE used to build the
cache for PROD image is "constraints" by default, while building
images in "build-images" workflow for regular PRs and canary
build uses "constraints-source-providers". The former is fine as
default for PROD image (as oppose to CI image we build PROD image
from released PyPI packages by default) but the latter is "proper"
for the CI cache, because there, the image is built out of local
packages prepared from sources.

Turns out that the CONSTRAINT_MODE parameter had a profound impact
on caching - because it was set before the
"install_packages_from_branch_tip" step and - in fact - even
before "install database clients" step, which caused our cache to
only work for the "base OS dependencies" - installing database
clients and installing airflow from branch tip (which works great
for CI image) had always been done in PRs because the layers in
cache with constraints env invalidated all subsequent layers.

This had no big impact before when testing usually took much longer
time - but since the testing has been vastly improved in apache#35160, now
PROD image building continues running even after test complete and
becomes the next frontier of optimization.

This PR optimizes PROD image building in two ways:

* caching is prepared with "source_providers" constraint mode, same
  as regular build

* the AIRFLOW_CONSTRAINT_MODE and related arguments are moved after
  installing database clients, so that this parameter does not
  impact their caching.
potiuk added a commit to potiuk/airflow that referenced this pull request Dec 22, 2023
When the DB/NonDB tests were introduced (apache#35160) new test types have
been added (separating various Python test types from generic
Operator test type). However we have not added matching of the python
operator and test files into the right selective unit test type. This
caused that when only `operators/python.py` and `tests/test_python` were
changed, then `Operators` test type was run but the specific Python *
test types were not run.

This PR fixes it for current test type (including also separated
Serialization test type) and for the future - instead of matching
selected test type we match all of them except the few that we
now are "special" ("Always, Core, Other, PlainAsserts").
potiuk added a commit that referenced this pull request Dec 22, 2023
…#36372)

When the DB/NonDB tests were introduced (#35160) new test types have
been added (separating various Python test types from generic
Operator test type). However we have not added matching of the python
operator and test files into the right selective unit test type. This
caused that when only `operators/python.py` and `tests/test_python` were
changed, then `Operators` test type was run but the specific Python *
test types were not run.

This PR fixes it for current test type (including also separated
Serialization test type) and for the future - instead of matching
selected test type we match all of them except the few that we
now are "special" ("Always, Core, Other, PlainAsserts").
potiuk added a commit that referenced this pull request Dec 30, 2023
…#36372)

When the DB/NonDB tests were introduced (#35160) new test types have
been added (separating various Python test types from generic
Operator test type). However we have not added matching of the python
operator and test files into the right selective unit test type. This
caused that when only `operators/python.py` and `tests/test_python` were
changed, then `Operators` test type was run but the specific Python *
test types were not run.

This PR fixes it for current test type (including also separated
Serialization test type) and for the future - instead of matching
selected test type we match all of them except the few that we
now are "special" ("Always, Core, Other, PlainAsserts").

(cherry picked from commit b0db1f9)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:CLI area:dev-env CI, pre-commit, pylint and other changes that do not change the behavior of the final code area:dev-tools area:Triggerer area:webserver Webserver related Issues type:misc/internal Changelog: Misc changes that should appear in change log
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants