You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/10/11 14:03:22 UTC
[GitHub] [airflow] Taragolis opened a new pull request, #26992: Replace `unittests` in helm chart tests by pure `pytest` [Wave-2]
Taragolis opened a new pull request, #26992:
URL: https://github.com/apache/airflow/pull/26992
Attempt to replace `unittests.TestCase` by native `pytest` classes for **all** charts tests.
All changes are straight forward:
- Get rid of `unittests.TestCase`
- Replace decorator `parameterized.expand` by `pytest.mark.parametrize`. I cant see any benefits if compare to builtin `pytest`.
- Rename ClassName**Test** to **Test**ClassName
- Replace `unittests.TestCase.assertRaises` by `pytest.raises`
Except two modules
- `tests/charts/test_pod_template_file.py` - move fixture out of the class
- `tests/charts/test_rbac.py` - `unittests.TestCase.assertCountEqual` has no equivalent in `pytest`, so i replace by compare sorted lists
----
Tested locally in breeze and did not have any differences if compare to current (local) main
_This PR (run 5 times)_
```
792 passed, 1 warning in 226.74s (0:03:46)
792 passed, 1 warning in 226.18s (0:03:46)
792 passed, 1 warning in 226.10s (0:03:46)
792 passed, 1 warning in 226.78s (0:03:46)
792 passed, 1 warning in 227.63s (0:03:47)
```
_Main (run 5 times)_
```
792 passed, 1 warning in 225.29s (0:03:45)
792 passed, 1 warning in 225.06s (0:03:45)
792 passed, 1 warning in 226.00s (0:03:46)
792 passed, 1 warning in 225.51s (0:03:45)
792 passed, 1 warning in 225.99s (0:03:45)
```
---
Just reminder that this PR remove all usage `pytest.mark.parametrize` within the charts tests so it can affect any of current [helm-chart](https://github.com/apache/airflow/pulls?q=is%3Apr+is%3Aopen+label%3Aarea%3Ahelm-chart) PRs which not merged yet and vice versa.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] jedcunningham merged pull request #26992: Replace `unittests` in helm chart tests by pure `pytest` [Wave-2]
Posted by GitBox <gi...@apache.org>.
jedcunningham merged PR #26992:
URL: https://github.com/apache/airflow/pull/26992
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #26992: Replace `unittests` in helm chart tests by pure `pytest` [Wave-2]
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #26992:
URL: https://github.com/apache/airflow/pull/26992#issuecomment-1291286082
❤️
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #26992: Replace `unittests` in helm chart tests by pure `pytest` [Wave-2]
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #26992:
URL: https://github.com/apache/airflow/pull/26992#issuecomment-1275969041
I think it makes sense to add them to the list. (While you’re at it, let’s also convert them to raw-strings (r-prefix) and fix the `API_CODEGEN_FILES` above—the `v1.yaml` pattern there is missing an escape (it should be `v1\.yaml` instead)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #26992: Replace `unittests` in helm chart tests by pure `pytest` [Wave-2]
Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #26992:
URL: https://github.com/apache/airflow/pull/26992#issuecomment-1274956090
Hmmm.... **Tests / Python unit tests for Helm chart (pull_request) Skipped** :eyes:
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] Taragolis commented on pull request #26992: Replace `unittests` in helm chart tests by pure `pytest` [Wave-2]
Posted by GitBox <gi...@apache.org>.
Taragolis commented on PR #26992:
URL: https://github.com/apache/airflow/pull/26992#issuecomment-1275960224
@uranusjr Thanks! Seems I found that breeze module
https://github.com/apache/airflow/blob/d9db89a2d923e1242c32c3c372ddd1c06d58cacb/dev/breeze/src/airflow_breeze/utils/selective_checks.py#L112-L116
https://github.com/apache/airflow/blob/d9db89a2d923e1242c32c3c372ddd1c06d58cacb/dev/breeze/src/airflow_breeze/utils/selective_checks.py#L467-L469
Should I extend the list within the `FileGroupForCi.HELM_FILES` group by add `^tests/charts`?
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #26992: Replace `unittests` in helm chart tests by pure `pytest` [Wave-2]
Posted by GitBox <gi...@apache.org>.
uranusjr commented on PR #26992:
URL: https://github.com/apache/airflow/pull/26992#issuecomment-1275941549
I think it’s because the tests are currently only run when something in the actual Helm Chart changes, but not when the _tests_ change. Probably should fix those in the CI config. (The configs are in `breeze/.../selective_checks.py` (don’t recall the exact path to the module)
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org