You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Taragolis (via GitHub)" <gi...@apache.org> on 2023/02/02 08:21:07 UTC
[GitHub] [airflow] Taragolis opened a new issue, #29305: Migrate remaining tests to `pytest`
Taragolis opened a new issue, #29305:
URL: https://github.com/apache/airflow/issues/29305
### Body
All tests for Apache Airflow are run using [`pytest`](http://doc.pytest.org/en/latest/). We have about 80 tests modules which still use [`unittests`](https://docs.python.org/3/library/unittest.html) runner for pytest , initial plan was "convert all unit tests to standard "asserts" semi-automatically, but this will be done later in Airflow 2.0 development phase. That will include setUp/tearDown/context managers and decorators.", see: [Writing Unit Tests](https://github.com/apache/airflow/blob/main/TESTING.rst#id4)
Personally I do not have a free time to complete this tasks, if someone would like to work on one (or many) parts of this Task just let us know in comments or _create a PR with reference to this Issue_.
For keep in track remaining tasks for migration to "native" pytest runner I split it by separate phases
Phase 1: Migration unittests to pytest
---
1. Replace unittests by pytest
2. Replace `@parameterized.expand` by `@pytest.mark.parametrize`
Example how it done previously: https://github.com/apache/airflow/pulls?q=is%3Apr+migrate+tests+pytest+is%3Aclosed+
**Always**
- [ ] Migrate remaining `tests/always` to pytest
**CLI**
- [ ] Migrate remaining `tests/cli` to pytest
**Other**
- [ ] Migrate remaining `tests/operators` to pytest
- [ ] Replace `@parameterized.expand` by `@pytest.mark.parametrize` in `tests/callbacks`
**Providers**
- Google (GCP)
+ [ ] Migrate remaining GCP operators tests `tests/providers/google/cloud/operators` to pytest
+ [ ] Migrate remaining GCP transfers operators tests `tests/providers/google/cloud/transfers` to pytest
+ [ ] Migrate remaining GCP sensors tests `tests/providers/google/cloud/sensors` to pytest
- Amazon (AWS)
+ [ ] Migrate remaining AWS tests `tests/providers/amazon/aws` to pytest
- Apache Flink
+ [ ] Migrate Apache Flink tests `tests/providers/apache/flink` to pytest
Phase 2: Ban unittests runner
---
It should be done after Phase 1.
This also include remove `parametrized` package, all features already exists in pytest core, see: [How to parametrize fixtures and test functions](https://docs.pytest.org/en/latest/how-to/parametrize.html#how-to-parametrize-fixtures-and-test-functions)
- [ ] Remove all unittests.TestCase usage in `tests/test_utils`, as far as I know right now [only one place](https://github.com/Taragolis/airflow/blob/ce677862be4a7de777208ba9ba9e62bcd0415393/tests/test_utils/asserts.py#L38) where we use it.
- [ ] Ban inheritance from `unittest.TestCase` it could be done by different ways:
+ Create collector hook which raise an error if detected unittests.TestCase, the opposite how builtin pytest plugin works, see: [unittest.py::pytest_pycollect_makeitem](https://github.com/pytest-dev/pytest/blob/4a46ee8bc957b06265c016cc837862447dde79d2/src/_pytest/unittest.py#L44-L57)
+ Develop pre-commit hook
- [ ] Disable builtin unittests plugin by add `-p no:unittest` in [`pytest.ini` adopts section](https://github.com/apache/airflow/blob/d80b583db07197c8c3d0549a805e83ceaaf10d52/pytest.ini#L19-L26), this step only disable discovering classes based on `unittests.TestCase`.
Some useful stuff
---
I use this command to find any remaining usage of `unittests.TestCase` and `parametrized`
```console
# All modules which potentially use unittests.TestCase
❯ grep -rl 'TestCase' ./tests | sort | uniq -c | sort -nr
# All modules which potentially use parameterized
❯ grep -rl 'from parameterized' ./tests | sort | uniq -c | sort -nr
# List of providers which potentially use unittests.TestCase
❯ grep -rl 'TestCase' tests/providers/ | cut -d"/" -f4 | sort | uniq -c | sort -nr
```
### Committer
- [X] I acknowledge that I am a maintainer/committer of the Apache Airflow project.
--
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.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk closed issue #29305: Migrate remaining tests to `pytest`
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk closed issue #29305: Migrate remaining tests to `pytest`
URL: https://github.com/apache/airflow/issues/29305
--
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] eladkal closed issue #29305: Migrate remaining tests to `pytest`
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal closed issue #29305: Migrate remaining tests to `pytest`
URL: https://github.com/apache/airflow/issues/29305
--
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 issue #29305: Migrate remaining tests to `pytest`
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1415948313
@Abhishek-kumar-samsung You could comment in PR, you really don't need to ping anywhere 🤣
I've have a full time job (as many other contributors of Airflow) so this is regular thing if you do not have feedback quick, but sooner or later I or someone else look on PR and related issues 👍
--
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] Abhishek-kumar-samsung commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "Abhishek-kumar-samsung (via GitHub)" <gi...@apache.org>.
Abhishek-kumar-samsung commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1415958282
Oh sorey @Taragolis this is my first time with open source thats why i am not more comfortable with procedure, will take care next time.
--
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] auvipy commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "auvipy (via GitHub)" <gi...@apache.org>.
auvipy commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1465088570
is this issue complete to be closed or need some more work?
--
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] JammUtkarsh commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "JammUtkarsh (via GitHub)" <gi...@apache.org>.
JammUtkarsh commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1442898723
Hey @Taragolis,
If I were to contribute to this issue, would I require the access to cloud environment like GCP or AWS?
ref: Phase 1/Provider
--
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] Abhishek-kumar-samsung commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "Abhishek-kumar-samsung (via GitHub)" <gi...@apache.org>.
Abhishek-kumar-samsung commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1454756772
@Taragolis regarding
`Remove all unittests.TestCase usage in tests/test_utils, as far as I know right now [only one place](https://github.com/Taragolis/airflow/blob/ce677862be4a7de777208ba9ba9e62bcd0415393/tests/test_utils/asserts.py#L38) where we use it.`
Is test_utils also any testcase file?
because when i ran it , it runs zero test cases.
--
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] eladkal commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1509777193
@auvipy are you raising PR for phase 2?
--
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] auvipy commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "auvipy (via GitHub)" <gi...@apache.org>.
auvipy commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1465998126
so I can handle the phase two
--
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] vincbeck commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "vincbeck (via GitHub)" <gi...@apache.org>.
vincbeck commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1536725317
You can mark phase 2.2 as done ;)
--
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] Abhishek-kumar-samsung commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "Abhishek-kumar-samsung (via GitHub)" <gi...@apache.org>.
Abhishek-kumar-samsung commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1415827431
I have migrated one file from unittest to pytest.
PR => [https://github.com/apache/airflow/pull/29354]
--
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] Abhishek-kumar-samsung commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "Abhishek-kumar-samsung (via GitHub)" <gi...@apache.org>.
Abhishek-kumar-samsung commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1415901206
@Taragolis did changes as you asked, regarding parameterized.expand in test cli files.
--
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] Abhishek-kumar-samsung commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "Abhishek-kumar-samsung (via GitHub)" <gi...@apache.org>.
Abhishek-kumar-samsung commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1435643544
@Taragolis tests in always are also completely migrated to pytest as per me. Only mock objects of unittest are still used, other things migrated.
So i think you can tick mark that task also.
--
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 issue #29305: Migrate remaining tests to `pytest`
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1443088873
Unit tests not required access to cloud environments.
For example configuration of Amazon Provider unit tests disable access to real AWS environment.
--
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 issue #29305: Migrate remaining tests to `pytest`
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1465971265
Also nice to have solve Phase 2
--
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 issue #29305: Migrate remaining tests to `pytest`
Posted by "Taragolis (via GitHub)" <gi...@apache.org>.
Taragolis commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1435711841
Thanks for contribution and help with migrations 💯 🥇
> Only mock objects of unittest are still used, other things migrated.
That is fine we do not have plan to remove them, even [`pytest-mock`](https://pypi.org/project/pytest-mock/) use internally `unittests.mock`
--
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] Abhishek-kumar-samsung commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "Abhishek-kumar-samsung (via GitHub)" <gi...@apache.org>.
Abhishek-kumar-samsung commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1415667316
I would like to work on this issue. I am checking the code for this.
--
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] eladkal commented on issue #29305: Migrate remaining tests to `pytest`
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on issue #29305:
URL: https://github.com/apache/airflow/issues/29305#issuecomment-1536756055
> Disable builtin unittests plugin by add -p no:unittest in [pytest.ini adopts section](https://github.com/apache/airflow/blob/d80b583db07197c8c3d0549a805e83ceaaf10d52/pytest.ini#L19-L26), this step only disable discovering classes based on unittests.TestCase.
About this step the file was removed in https://github.com/apache/airflow/pull/30315
--------
thank you @vincbeck
--
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