Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1055,10 +1055,12 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
- name: "Test Offline SQL generation"
run: ./scripts/ci/testing/run_offline_sql_test.sh
- name: "Tests: ${{needs.build-info.outputs.test-types}}"
run: ./scripts/ci/testing/ci_run_airflow_testing.sh
run: breeze testing tests
--run-parallel-test
env:
PR_LABELS: "${{ needs.build-info.outputs.pull-request-labels }}"
IMAGE_TAG: ${{ env.IMAGE_TAG_FOR_THE_BUILD }}
TEST_TYPES: ${{needs.build-info.outputs.test-types}}
- name: "Upload airflow logs"
uses: actions/upload-artifact@v3
if: failure()
Expand Down Expand Up @@ -1126,10 +1128,12 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
- name: "Test downgrade"
run: ./scripts/ci/testing/run_downgrade_test.sh
- name: "Tests: ${{needs.build-info.outputs.test-types}}"
run: ./scripts/ci/testing/ci_run_airflow_testing.sh
run: breeze testing tests
--run-parallel-test
env:
PR_LABELS: "${{ needs.build-info.outputs.pull-request-labels }}"
IMAGE_TAG: ${{ env.IMAGE_TAG_FOR_THE_BUILD }}
TEST_TYPES: ${{needs.build-info.outputs.test-types}}
- name: "Upload airflow logs"
uses: actions/upload-artifact@v3
if: failure()
Expand Down Expand Up @@ -1197,10 +1201,12 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
- name: "Test downgrade"
run: ./scripts/ci/testing/run_downgrade_test.sh
- name: "Tests: ${{needs.build-info.outputs.test-types}}"
run: ./scripts/ci/testing/ci_run_airflow_testing.sh
run: breeze testing tests
--run-parallel-test
env:
PR_LABELS: "${{ needs.build-info.outputs.pull-request-labels }}"
IMAGE_TAG: ${{ env.IMAGE_TAG_FOR_THE_BUILD }}
TEST_TYPES: ${{needs.build-info.outputs.test-types}}
- name: "Upload airflow logs"
uses: actions/upload-artifact@v3
if: failure()
Expand Down Expand Up @@ -1266,10 +1272,12 @@ ${{ hashFiles('.pre-commit-config.yaml') }}"
- name: "Test downgrade"
run: ./scripts/ci/testing/run_downgrade_test.sh
- name: "Tests: ${{needs.build-info.outputs.test-types}}"
run: ./scripts/ci/testing/ci_run_airflow_testing.sh
run: breeze testing tests
--run-parallel-test
env:
PR_LABELS: "${{ needs.build-info.outputs.pull-request-labels }}"
IMAGE_TAG: ${{ env.IMAGE_TAG_FOR_THE_BUILD }}
TEST_TYPES: ${{needs.build-info.outputs.test-types}}
- name: "Upload airflow logs"
uses: actions/upload-artifact@v3
if: failure()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,6 @@ def run_build(ci_image_params: BuildCiParams) -> None:
@option_verify
@option_wait_for_image
@option_image_tag_for_pulling
@option_include_success_outputs
Copy link
Member

Choose a reason for hiding this comment

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

This seems like conflict resolution problem too.

@option_tag_as_latest
@click.argument('extra_pytest_args', nargs=-1, type=click.UNPROCESSED)
def pull(
Expand Down
89 changes: 88 additions & 1 deletion dev/breeze/src/airflow_breeze/commands/testing_commands.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,16 @@
option_github_repository,
option_image_name,
option_image_tag_for_running,
option_include_success_outputs,
option_integration,
option_mount_sources,
option_mssql_version,
option_mysql_version,
option_parallelism,
option_postgres_version,
option_python,
option_run_parallel_test,
option_skip_cleanup,
option_verbose,
)
from airflow_breeze.utils.console import get_console, message_type_from_return_code
Expand All @@ -55,6 +59,7 @@
get_env_variables_for_docker_commands,
perform_environment_checks,
)
from airflow_breeze.utils.parallel import check_async_run_results, run_with_pool
from airflow_breeze.utils.run_tests import run_docker_compose_tests
from airflow_breeze.utils.run_utils import RunCommandResult, run_command

Expand Down Expand Up @@ -189,6 +194,68 @@ def run_with_progress(
return result


def run_tests(
env_variables: Dict[str, str], dry_run: bool, verbose: bool, test_type: str, extra_pytest_args: Tuple
) -> Tuple[int, str]:
env_variables['TEST_TYPE'] = test_type
perform_environment_checks(verbose=verbose)
cmd = ['docker-compose', 'run', '--service-ports', '--rm', 'airflow']
cmd.extend(list(extra_pytest_args))
test_result = run_command(
cmd,
verbose=verbose,
dry_run=dry_run,
env=env_variables,
)
return (
test_result.returncode,
f"Running tests {test_type}",
)


Copy link
Contributor Author

Choose a reason for hiding this comment

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

@potiuk Share your review in this code when you get time. I didn't yet add the parser part. But your reviews until this part would be useful. Thanks

def run_tests_in_parallel(
env_variables: Dict[str, str],
test_type_list: List[str],
include_success_outputs: bool,
skip_cleanup: bool,
parallelism: int,
dry_run: bool,
verbose: bool,
extra_pytest_args: Tuple,
):
"""Run tests in parallel"""
with ci_group(f"Running tests for the type {test_type_list}"):
# get_console().print(
# f"\n[info]Running tests with parallelism = {parallelism} "
# f"for the test type: {test_type_list}[/]"
# )
all_params = [f"Tests {test_type}" for test_type in test_type_list]
with run_with_pool(
parallelism=parallelism,
all_params=all_params,
) as (pool, outputs):
results = [
pool.apply_async(
run_tests,
kwds={
"env_variables": env_variables,
"dry_run": dry_run,
"verbose": verbose,
"test_type": test_type,
"extra_pytest_args": extra_pytest_args,
},
)
for test_type in test_type_list
]
check_async_run_results(
results=results,
success="All tests run correctly",
outputs=outputs,
include_success_outputs=include_success_outputs,
skip_cleanup=skip_cleanup,
)


@testing.command(
name='tests',
help="Run the specified unit test targets.",
Expand Down Expand Up @@ -218,6 +285,7 @@ def run_with_progress(
"tests should be run - for example --test-type \"Providers[airbyte,http]\"",
default="All",
type=NotVerifiedBetterChoice(ALLOWED_TEST_TYPE_CHOICES),
envvar='TEST_TYPES',
)
@click.option(
"--test-timeout",
Expand All @@ -226,6 +294,10 @@ def run_with_progress(
show_default=True,
)
@option_db_reset
@option_run_parallel_test
@option_parallelism
@option_include_success_outputs
@option_skip_cleanup
@click.argument('extra_pytest_args', nargs=-1, type=click.UNPROCESSED)
def tests(
dry_run: bool,
Expand All @@ -241,8 +313,12 @@ def tests(
test_type: str,
test_timeout: str,
db_reset: bool,
image_tag: str | None,
run_parallel_test: bool,
parallelism: int,
image_tag: Optional[str],
mount_sources: str,
skip_cleanup: bool,
include_success_outputs: bool,
Copy link
Member

Choose a reason for hiding this comment

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

I think we can remove limit_progress_output parameter altogether (from both helm_tests and testing). Once we get parallel running, the limit_progess_output should be always enabled when run in parallel and disabled when not, so we do not need another flag there.

):
exec_shell_params = ShellParams(
verbose=verbose,
Expand Down Expand Up @@ -293,6 +369,17 @@ def tests(
verbose=verbose,
dry_run=dry_run,
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@potiuk I have rebased the changes in main and currently, I have changes to run the test in parallel and the changes you have added include showing the limited progress.

I need to add the option to show the limited progress in parallel tests alone or both parallel and usual test flow?

Copy link
Member

Choose a reason for hiding this comment

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

I think the "limited" output should be always enabled when parallell is used

Copy link
Member

Choose a reason for hiding this comment

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

I just opened a PR to remove the option in Helm tests here: #26144

elif run_parallel_test:
run_tests_in_parallel(
env_variables=env_variables,
test_type_list=ALLOWED_TEST_TYPE_CHOICES,
include_success_outputs=include_success_outputs,
skip_cleanup=skip_cleanup,
parallelism=parallelism,
dry_run=dry_run,
verbose=verbose,
extra_pytest_args=extra_pytest_args if extra_pytest_args is not None else (),
)
else:
result = run_command(cmd, verbose=verbose, dry_run=dry_run, env=env_variables, check=False)
sys.exit(result.returncode)
Expand Down
3 changes: 0 additions & 3 deletions dev/breeze/src/airflow_breeze/global_constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,10 @@ class SelectiveUnitTestTypes(Enum):

ALLOWED_TEST_TYPE_CHOICES = [
"All",
"Always",
*all_selective_test_types(),
"Helm",
"Postgres",
"MySQL",
"Integration",
"Other",
"Quarantine",
]

Expand Down
6 changes: 6 additions & 0 deletions dev/breeze/src/airflow_breeze/utils/common_options.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,12 @@ def _set_default_from_parent(ctx: click.core.Context, option: click.core.Option,
is_flag=True,
envvar='RUN_IN_PARALLEL',
)
option_run_parallel_test = click.option(
'--run-parallel-test',
help="Run the operation in parallel on all of Test types",
is_flag=True,
envvar='RUN_PARALLEL_TEST',
)
option_parallelism = click.option(
'--parallelism',
help="Maximum number of processes to use while running the operation in parallel.",
Expand Down
8 changes: 4 additions & 4 deletions images/breeze/output-commands-hash.txt
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,9 @@ ci:resource-check:0fb929ac3496dbbe97acfe99e35accd7
ci:selective-check:d4e3c250cd6f2b0040fbe6557fa423f6
ci:31566cdcdde216086f559215223b2378
ci-image:build:d39a25675e6b74af9bbb1fc2582aacc5
ci-image:pull:8aca8679e6030ad0d6e59216af40c0b3
ci-image:pull:fdde25906ad4c666db606201be44c240
ci-image:verify:a2daeaa820c0baca31da2737929b38b9
ci-image:b1c7a3c6dfa72b127fac559dcfdbb0d3
ci-image:310bac16f2c61789453569a4f7901595
cleanup:9bf46a1dfd9db4fe13a1c233ad1bb96b
compile-www-assets:23675c1862d0968cbff6ab6f1d93d488
exec:89b81bc34d45b0fe6653a6db5482258c
Expand Down Expand Up @@ -53,5 +53,5 @@ static-checks:425cd78507278494e345fb7648260c24
stop:8ebd8a42f1003495d37b884de5ac7ce6
testing:docker-compose-tests:3e07be65e30219930d3c62a593dd8c6a
testing:helm-tests:403231f0a94b261f9c7aae8aea03ec50
testing:tests:32deda30f3899e8ae6e241238f990d68
testing:e747ece268ba502c106924eb2f46c550
testing:tests:418233cc2313a34da99fb41a841fe0cf
testing:416c94fc5fce116f7f79d296484dfa1b
Loading