kaxil opened a new pull request, #73500: URL: https://github.com/apache/airflow/pull/73500
`providers/standard/tests/unit/standard/operators/test_python.py` held 138 tests that each spawn a real interpreter subprocess, often after creating a virtualenv. On the public CI runners they take 9 to 14 seconds each, and they are why the `Providers[standard]` bucket is the longest in the provider test jobs (about 22 of its 25 minutes in the compat jobs). Most of them were exercising the same code more than once. **The shared subprocess round trip ran on four classes.** Argument pickling, string args, env var handling, error propagation and result return all live in `_BasePythonVirtualenvOperator._execute_python_callable_in_subprocess`, which `PythonVirtualenvOperator` and `ExternalPythonOperator` both use unchanged. The base test class ran 13 such tests on both, plus the two branch variants. They now live in a `_SubprocessBehaviourTests` mixin applied to `TestExternalPythonOperator` only, which needs no virtualenv. The virtualenv class keeps everything that is actually about virtualenvs: requirements, caching, index URLs, system site packages, serializer installation, context serialization. **`test_on_skip_exit_code` had 17 cases per class for two lines of code.** Thirteen of them only varied how the argument was spelled (`100`, `[100]`, `(100,)`, `None`), which is constructor normalisation at `python.py:533`. That is now a parametrized test on the operator attribute with no subprocess, still run on all four classes. Four subprocess cases remain: exit 0 succeeds, unlisted code fails, listed code skips, `skip_on_exit_code=0` skips. **The branch classes' env var and context tests could not fail for the reason their names suggest.** `BaseTestBranchPythonVirtualenvOperator` overrode six inherited tests to expect the branch validation error. The callable returned the env var value, and the branch operator rejects any string that is not a task id, so the env var content never affected the outcome. `test_environment_variables_with_inherit_env_false` asserted a bare `AirflowException`, which a `KeyError` in the subprocess and the validation error both satisfy. Removed. Invalid return values are still covered by `test_return_false`, `test_raise_exception_on_no_accepted_type_return` and `test_raise_exception_on_invalid_task_id`. **The `default` serializer parameter was the `pickle` parameter.** `serializer or "pickle"` at `python.py:522`. Removed from the seven lists that had both. Measured locally in breeze, same machine, `-k "Virtualenv or ExternalPython"`: | | Tests | Wall clock | |---|---|---| | Before | 203 passed, 18 skipped | 351s | | After | 151 passed, 10 skipped | 170s | The decorator tests in `providers/standard/tests/unit/standard/decorators/` copy most of the operator tests as well, but that is a separate judgement and is not in this PR. --- * Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information. Note: commit author/co-author name and email in commits become permanently public when merged. * For fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed. * When adding dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x). * For significant user-facing changes create newsfragment: `{pr_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments). You can add this file in a follow-up commit after the PR is created so you know the PR number. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
