Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Restore providers' unit tests marked as integration tests #39768

Closed
wants to merge 1 commit into from

Conversation

shahar1
Copy link
Contributor

@shahar1 shahar1 commented May 22, 2024

As I prepared to implement mssql integration tests (#38577), I noticed that existing unit tests had been moved to tests/integration (#38577, #28170), even though they are just pure unit tests. There's no setup_method for connection with an external service; everything is nicely mocked, and they run OK on Breeze locally (maybe a bit slow, but that's for another time)...why leave them out? :)
This PR relocates the unit tests back to their original directory, and also fixes a path in the test_project_structure.py (the original paths for the unit tests above were left in this file as is).
Also, test_bigquery_to_mssql.py was duplicated in the original path and in the providers' integration tests - I assume that by mistake or as a blueprint for the integration tests, but for now it's better be deleted to avoid confusion.

cc: @potiuk, @Taragolis


^ 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.

@boring-cyborg boring-cyborg bot added area:providers provider:apache-hive provider:google Google (including GCP) related issues labels May 22, 2024
@shahar1 shahar1 changed the title Restore unit tests marked as integration tests Restore providers' unit tests marked as integration tests May 22, 2024
@eladkal eladkal requested a review from Taragolis May 23, 2024 05:30
Copy link
Contributor

@Taragolis Taragolis left a comment

Choose a reason for hiding this comment

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

Some history: It moved into integration because it have unsupported marker @pytest.mark.backend("mssql"): #38530, there was no check for unsupported DB Backends at that time.
That not a case anymore, because right now we check for supported backends: mysql, postgres, sqlite

All tests which marked as @pytest.mark.backend also automatically (implicitly) marked as @pytest.mark.db_test so it never run at non-db environment. Some modules or individual test might be required to be marked with pytest.mark.db_test, you could check locally in breeze which moved tests required this marker https://github.com/apache/airflow/blob/main/contributing-docs/testing/unit_tests.rst#db-and-non-db-tests

breeze shell --backend none

root@a2b2d941fc72:/opt/airflow# pytest tests --skip-db-tests tests/providers/google/cloud/transfers/test_mssql_to_gcs.py tests/providers/google/cloud/transfers/test_mssql_to_gcs.py tests/providers/apache/hive/transfers/test_mssql_to_hive.py tests/providers/google/cloud/transfers/test_bigquery_to_mssql.py

@Taragolis Taragolis added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label May 23, 2024
@shahar1
Copy link
Contributor Author

shahar1 commented May 24, 2024

Some history: It moved into integration because it have unsupported marker @pytest.mark.backend("mssql"): #38530, there was no check for unsupported DB Backends at that time. That not a case anymore, because right now we check for supported backends: mysql, postgres, sqlite

All tests which marked as @pytest.mark.backend also automatically (implicitly) marked as @pytest.mark.db_test so it never run at non-db environment. Some modules or individual test might be required to be marked with pytest.mark.db_test, you could check locally in breeze which moved tests required this marker https://github.com/apache/airflow/blob/main/contributing-docs/testing/unit_tests.rst#db-and-non-db-tests

❯ breeze shell --backend none

root@a2b2d941fc72:/opt/airflow# pytest tests --skip-db-tests tests/providers/google/cloud/transfers/test_mssql_to_gcs.py tests/providers/google/cloud/transfers/test_mssql_to_gcs.py tests/providers/apache/hive/transfers/test_mssql_to_hive.py tests/providers/google/cloud/transfers/test_bigquery_to_mssql.py

I ran the tests with these commands and all ran smoothly.
As I previously stated, as of the present time, these are simple unit tests - so there shouldn't be a reason for them not to work without a backend.
I'll start working on the mssql integration tests soon.

@shahar1 shahar1 requested a review from Taragolis May 24, 2024 12:45
@shahar1 shahar1 marked this pull request as draft May 25, 2024 05:53
@shahar1 shahar1 marked this pull request as ready for review May 25, 2024 05:54
@shahar1
Copy link
Contributor Author

shahar1 commented May 25, 2024

Closing to avoid conflicts with #39831 (I implemented the unit tests relocation there)

@shahar1 shahar1 closed this May 25, 2024
@shahar1 shahar1 deleted the revert-38530 branch June 12, 2024 13:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
area:providers changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) provider:apache-hive provider:google Google (including GCP) related issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants