Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[AIRFLOW-6058] Running tests with pytest #6472

Merged
merged 1 commit into from
Dec 5, 2019

Conversation

turbaszek
Copy link
Member

@turbaszek turbaszek commented Oct 30, 2019

Make sure you have checked all steps below.

Jira

Description

  • Here are some details about my PR, including screenshots of any UI changes:

This PR switches nosetest to pytest. I tried to keep required changes to minimum.
What I have already learned is that:

  • many tests depend on database
  • order of tests is important
  • there are tests that create side effects in logging setup, local settings, config.

I am open to any suggestions and help.

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:

Commits

  • My commits all reference Jira issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters (not including Jira issue reference)
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

Documentation

  • In case of new functionality, my PR adds documentation that describes how to use it.
    • All the public functions and the classes in the PR contain docstrings that explain what it does
    • If you implement backwards incompatible changes, please leave a note in the Updating.md so we can assign it to a appropriate release

setup.py Outdated Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
@feluelle
Copy link
Member

feluelle commented Nov 1, 2019

Oh just forgot it is not ready for review, sry 😬

@feluelle
Copy link
Member

feluelle commented Nov 1, 2019

I can't wait to finally have pytest 😁

@akki
Copy link
Contributor

akki commented Nov 20, 2019

Is there any place (JIRA ticket or discussion) where I can know why we want to move towards pytest?

@turbaszek
Copy link
Member Author

turbaszek commented Nov 20, 2019

@turbaszek turbaszek force-pushed the pytest-poc branch 4 times, most recently from e62a969 to 6f8cc80 Compare November 25, 2019 14:59
@turbaszek turbaszek changed the title [AIRFLOW-YYYY][POC] Running tests with pytest [AIRFLOW-6058] Running tests with pytest Nov 25, 2019
airflow/utils/log/colored_log.py Show resolved Hide resolved
airflow/www/views.py Outdated Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
airflow/www/views.py Outdated Show resolved Hide resolved
@turbaszek turbaszek force-pushed the pytest-poc branch 2 times, most recently from dfcaebd to fa61eeb Compare November 25, 2019 19:58
tests/conftest.py Outdated Show resolved Hide resolved
@turbaszek turbaszek marked this pull request as ready for review November 25, 2019 21:21
Copy link
Member

@potiuk potiuk left a comment

Choose a reason for hiding this comment

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

Some small comments.

airflow/www/views.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
setup.py Show resolved Hide resolved
tests/conftest.py Outdated Show resolved Hide resolved
tests/pytest.ini Outdated Show resolved Hide resolved
tests/task/task_runner/test_standard_task_runner.py Outdated Show resolved Hide resolved
tests/test_logging_config.py Show resolved Hide resolved
tests/www/test_views.py Show resolved Hide resolved
setup.py Outdated Show resolved Hide resolved
tests/__init__.py Outdated Show resolved Hide resolved
tests/pytest.ini Outdated Show resolved Hide resolved
setup.py Show resolved Hide resolved
tests/pytest.ini Outdated Show resolved Hide resolved
tests/task/task_runner/test_standard_task_runner.py Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Nov 26, 2019

Codecov Report

Merging #6472 into master will increase coverage by 0.73%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6472      +/-   ##
==========================================
+ Coverage    83.9%   84.64%   +0.73%     
==========================================
  Files         668      669       +1     
  Lines       37687    37703      +16     
==========================================
+ Hits        31622    31912     +290     
+ Misses       6065     5791     -274
Impacted Files Coverage Δ
airflow/www/views.py 75.93% <ø> (-0.85%) ⬇️
airflow/utils/log/colored_log.py 89.58% <57.14%> (-3.6%) ⬇️
airflow/executors/sequential_executor.py 56% <0%> (-44%) ⬇️
...low/ti_deps/deps/exec_date_after_start_date_dep.py 80% <0%> (-10%) ⬇️
airflow/executors/executor_loader.py 59.09% <0%> (-6.82%) ⬇️
airflow/utils/log/es_task_handler.py 88.07% <0%> (-3.67%) ⬇️
airflow/utils/sqlalchemy.py 89.83% <0%> (-3.39%) ⬇️
airflow/utils/log/s3_task_handler.py 98.5% <0%> (-1.5%) ⬇️
airflow/executors/base_executor.py 96.05% <0%> (-1.32%) ⬇️
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e5ff4a1...b0514d5. Read the comment docs.

@turbaszek turbaszek force-pushed the pytest-poc branch 2 times, most recently from d939d60 to 88b7c0a Compare November 26, 2019 18:55
@turbaszek turbaszek force-pushed the pytest-poc branch 3 times, most recently from 9729f0f to dc84e09 Compare December 4, 2019 13:11
This commit runs Airflow's test suite using pytest.

fixup! [AIRFLOW-6058] Running tests with pytest

fixup! [AIRFLOW-6058] Running tests with pytest

fixup! [AIRFLOW-6058] Running tests with pytest

fixup! [AIRFLOW-6058] Running tests with pytest

fixup! [AIRFLOW-6058] Running tests with pytest

fixup! [AIRFLOW-6058] Running tests with pytest

fixup! [AIRFLOW-6058] Running tests with pytest

fixup! [AIRFLOW-6058] Running tests with pytest

fixup! [AIRFLOW-6058] Running tests with pytest

fixup! [AIRFLOW-6058] Running tests with pytest

fixup! [AIRFLOW-6058] Running tests with pytest
@turbaszek turbaszek requested a review from potiuk December 4, 2019 13:15
@potiuk potiuk merged commit e61025e into apache:master Dec 5, 2019
# Initialize kerberos
kerberos = os.environ.get("KRB5_KTNAME")
if kerberos:
subprocess.call(["kinit", "-kt", kerberos, "airflow"])
Copy link
Member

Choose a reason for hiding this comment

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

This should probably be check_call - if the kinit fails this won't throw an error.

Copy link
Member

Choose a reason for hiding this comment

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

Good call. @nuclearpinguin -> something as a follow up (including the setting of pytest as default test runner.

potiuk pushed a commit that referenced this pull request Dec 10, 2019
This commit runs Airflow's test suite using pytest.

(cherry picked from commit e61025e)
potiuk pushed a commit that referenced this pull request Dec 10, 2019
This commit runs Airflow's test suite using pytest.

(cherry picked from commit e61025e)
potiuk pushed a commit that referenced this pull request Dec 10, 2019
This commit runs Airflow's test suite using pytest.

(cherry picked from commit e61025e)
potiuk added a commit to potiuk/airflow that referenced this pull request Dec 10, 2019
potiuk pushed a commit that referenced this pull request Dec 10, 2019
This commit runs Airflow's test suite using pytest.

(cherry picked from commit e61025e)
potiuk pushed a commit that referenced this pull request Dec 11, 2019
This commit runs Airflow's test suite using pytest.

(cherry picked from commit e61025e)
potiuk pushed a commit that referenced this pull request Dec 11, 2019
This commit runs Airflow's test suite using pytest.

(cherry picked from commit e61025e)
kaxil pushed a commit that referenced this pull request Dec 12, 2019
This commit runs Airflow's test suite using pytest.

(cherry picked from commit e61025e)
kaxil pushed a commit that referenced this pull request Dec 17, 2019
This commit runs Airflow's test suite using pytest.

(cherry picked from commit e61025e)
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
This commit runs Airflow's test suite using pytest.
@turbaszek turbaszek deleted the pytest-poc branch March 14, 2020 23:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants