Skip to content

Improved unit tests for open_maybe_zipped function.#14114

Merged
ashb merged 4 commits intoapache:masterfrom
levahim:master
Feb 17, 2021
Merged

Improved unit tests for open_maybe_zipped function.#14114
ashb merged 4 commits intoapache:masterfrom
levahim:master

Conversation

@levahim
Copy link
Contributor

@levahim levahim commented Feb 6, 2021

This PR is to address requests formulated in the comments to PR #13984. Namely:

  1. As requested by @mik-laj the airflow.utils.file unit tests moved out of test_dag_processing.py into its own module test_file.py.
  2. As requested by @ashb the open_maybe_zipped function unit test, instead of relying on mocks, now actually reads a test file contents, zipped and plain, and verifies that the returned file content is string in both cases.

@github-actions
Copy link

github-actions bot commented Feb 6, 2021

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@levahim
Copy link
Contributor Author

levahim commented Feb 6, 2021

The pylint and k8s test errors appear to be completely unrelated to what's in this PR. Seems to be something that came from master.

@github-actions
Copy link

github-actions bot commented Feb 6, 2021

The Workflow run is cancelling this PR. It has some failed jobs matching ^Pylint$,^Static checks,^Build docs$,^Spell check docs$,^Backport packages$,^Provider packages,^Checks: Helm tests$,^Test OpenAPI*.

@github-actions
Copy link

github-actions bot commented Feb 8, 2021

The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.

@github-actions github-actions bot added the full tests needed We need to run full set of tests for this PR to merge label Feb 8, 2021
@mik-laj
Copy link
Member

mik-laj commented Feb 10, 2021

CI are sad. Can you do a rebase?

@github-actions
Copy link

The Workflow run is cancelling this PR. Building images for the PR has failed. Follow the the workflow link to check the reason.

@levahim
Copy link
Contributor Author

levahim commented Feb 13, 2021

The CI pipelines cancelled with error: "The self-hosted runner: Airflow Runner 60 lost communication with the server. Verify the machine is running and has a healthy network connection. Anything in your workflow that terminates the runner process, starves it for CPU/Memory, or blocks its network access can cause this error." I don't think it's under my control. Can you restart the build? (Or let me know if I can restart it e.g. with special comments.)

@ashb ashb merged commit faa6668 into apache:master Feb 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full tests needed We need to run full set of tests for this PR to merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants