Skip to content

[AIRFLOW-862] Fix Unit Tests for DaskExecutor#2076

Closed
jlowin wants to merge 1 commit intoapache:masterfrom
jlowin:fix-dask-tests
Closed

[AIRFLOW-862] Fix Unit Tests for DaskExecutor#2076
jlowin wants to merge 1 commit intoapache:masterfrom
jlowin:fix-dask-tests

Conversation

@jlowin
Copy link
Copy Markdown
Member

@jlowin jlowin commented Feb 13, 2017

Unit tests were inadvertently disabled for DaskExecutor added in #2067.

@bolkedebruin in reviewing the tests, I fixed the ImportError that caused them to be disabled, streamlined them slightly, and removed one that turned out to be useless without a BackfillJob checking the executor's event buffer -- that case is already covered by the Dask integration test.

Dear Airflow Maintainers,

Please accept this PR that addresses the following issues:

@codecov-io
Copy link
Copy Markdown

codecov-io commented Feb 14, 2017

Codecov Report

Merging #2076 into master will increase coverage by 0.31%.
The diff coverage is 60%.

@@            Coverage Diff             @@
##           master    #2076      +/-   ##
==========================================
+ Coverage   66.67%   66.98%   +0.31%     
==========================================
  Files         142      142              
  Lines       10725    10724       -1     
==========================================
+ Hits         7151     7184      +33     
+ Misses       3574     3540      -34
Impacted Files Coverage Δ
airflow/executors/dask_executor.py 81.39% <60%> (+81.39%)
airflow/task_runner/bash_task_runner.py 93.33% <ø> (-6.67%)
airflow/utils/helpers.py 41.08% <ø> (-2.48%)
airflow/jobs.py 73.07% <ø> (ø)
airflow/models.py 86.24% <ø> (+0.1%)
airflow/executors/base_executor.py 94.64% <ø> (+3.57%)

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 facb6a6...3de227c. Read the comment docs.

@jlowin jlowin force-pushed the fix-dask-tests branch 2 times, most recently from 012a52f to 5fdfb7f Compare February 14, 2017 16:03
@bolkedebruin
Copy link
Copy Markdown
Contributor

@jlowin looks good, can you investigate why postgres is failing on py3? Maybe unrelated, not sure. and maybe rebase to see if decrease in coverage and landscape errors will go away.

@jlowin
Copy link
Copy Markdown
Member Author

jlowin commented Feb 18, 2017

@bolke weird error... seems unrelated. I'm guessing the coverage decrease is because the unit test I deleted had a failed task in it and the remaining one doesn't. I don't really see what else could have caused that (the two unit tests were almost identical otherwise). I'm not sure what landscape's issue is either, when I click details it says no problems.

I'm going to rebase and push and let's see what happens!

Unit tests were inadvertently disabled for
DaskExecutor
@bolkedebruin
Copy link
Copy Markdown
Contributor

LGTM

@asfgit asfgit closed this in fe78816 Feb 19, 2017
@jlowin jlowin deleted the fix-dask-tests branch February 19, 2017 13:14
alekstorm pushed a commit to alekstorm/incubator-airflow that referenced this pull request Jun 1, 2017
Unit tests were inadvertently disabled for
DaskExecutor

Closes apache#2076 from jlowin/fix-dask-tests
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.

3 participants