Skip to content

Comments

migrating from unittest to pytest tests cli file#29354

Merged
Taragolis merged 3 commits intoapache:mainfrom
Abhishek-kumar-ISM:migrating_from_unittest_to_pytest_tests_cli
Feb 3, 2023
Merged

migrating from unittest to pytest tests cli file#29354
Taragolis merged 3 commits intoapache:mainfrom
Abhishek-kumar-ISM:migrating_from_unittest_to_pytest_tests_cli

Conversation

@Abhishek-kumar-ISM
Copy link
Contributor

@Abhishek-kumar-ISM Abhishek-kumar-ISM commented Feb 3, 2023

I have migrated one test file from unittest to pytest. All test cases of this file are running normally as earlier.

image

related: #29305

@boring-cyborg
Copy link

boring-cyborg bot commented Feb 3, 2023

Congratulations on your first Pull Request and welcome to the Apache Airflow community! If you have any issues or are unsure about any anything please check our Contribution Guide (https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst)
Here are some useful points:

  • Pay attention to the quality of your code (ruff, mypy and type annotations). Our pre-commits will help you with that.
  • In case of a new feature add useful documentation (in docstrings or in docs/ directory). Adding a new operator? Check this short guide Consider adding an example DAG that shows how users should use it.
  • Consider using Breeze environment for testing locally, it's a heavy docker but it ships with a working Airflow and a lot of integrations.
  • Be patient and persistent. It might take some time to get a review or get the final approval from Committers.
  • Please follow ASF Code of Conduct for all communication including (but not limited to) comments on Pull Requests, Mailing list and Slack.
  • Be sure to read the Airflow Coding style.
    Apache Airflow is a community-driven project and together we are making it better 🚀.
    In case of doubts contact the developers at:
    Mailing List: dev@airflow.apache.org
    Slack: https://s.apache.org/airflow-slack

@Taragolis Taragolis self-requested a review February 3, 2023 13:02
@Taragolis Taragolis added changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) type:misc/internal Changelog: Misc changes that should appear in change log labels Feb 3, 2023
@Taragolis
Copy link
Contributor

There is also one file exists in cli which use parameterized.expand:

@parameterized.expand(
[
("--ignore-all-dependencies",),
("--ignore-depends-on-past",),
("--ignore-dependencies",),
("--force",),
],
)
def test_cli_run_invalid_raw_option(self, option: str):
with pytest.raises(

It would be nice to replace it by @pytest.mark.parametrize so we could complete CLI tests in one go.

This example could be helpful:

@pytest.mark.parametrize("test_value", ["value", ""])
def test_var_with_encryption_rotate_fernet_key(self, test_value):
"""

@Abhishek-kumar-ISM
Copy link
Contributor Author

@Taragolis Yes i will do

@Abhishek-kumar-ISM
Copy link
Contributor Author

Abhishek-kumar-ISM commented Feb 3, 2023

@Taragolis completed the changes you said.

file running fine in which i did changes.

@Taragolis
Copy link
Contributor

Let's wait till CI green.
I guess it will fail on static checks, i think you forgot to remove unused import.

General recommendation it is setup pre-commit, this step will save ton of your time, see STATIC CHECKS

@Abhishek-kumar-ISM
Copy link
Contributor Author

Okay, i will remove all unused imports.

@Abhishek-kumar-ISM
Copy link
Contributor Author

Abhishek-kumar-ISM commented Feb 3, 2023

@Taragolis
removed all unused imports, all checks are passed.

@Taragolis Taragolis merged commit 59cde3f into apache:main Feb 3, 2023
@boring-cyborg
Copy link

boring-cyborg bot commented Feb 3, 2023

Awesome work, congrats on your first merged pull request!

@Abhishek-kumar-ISM Abhishek-kumar-ISM deleted the migrating_from_unittest_to_pytest_tests_cli branch February 4, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:CLI changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) type:misc/internal Changelog: Misc changes that should appear in change log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants