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

Add mising common.io test_file_transfer.py #35457

Merged
merged 1 commit into from
Nov 5, 2023

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Nov 5, 2023

Since our test to detect missing tests was broken, lack of the test_file_transfer.py (among many others) has not been detected before.

This PR adds a simple test that mocks out ObjectStoragePath and tests that they are initialized properly and that execute method executes copy on the source Object poth (but also does not call copy on the target one).

Part of #35442

Also unblocks #35241 which fails becasuse Pytest fails when it sees no tests collected when Providers[common.io] TEST_TYPE is used.


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

Since our test to detect missing tests was broken, lack of the
test_file_transfer.py (among many others) has not been detected
before.

This PR adds a simple test that mocks out ObjectStoragePath and
tests that they are initialized properly and that execute method
executes `copy` on the source Object poth (but also does not call
copy on the target one).

Part of apache#35442

Also unblocks apache#35241 which fails becasuse Pytest fails when it sees
no tests collected when `Providers[common.io]'` TEST_TYPE is used.
@bolkedebruin
Copy link
Contributor

Ty 🙏

potiuk added a commit to potiuk/airflow that referenced this pull request Nov 5, 2023
When we decide which tests should be run we err on a safe side and
when we see that some files were modified that we cannot classify
to specific providers, API, CLI, WWW area, we err on a safe side
and assume that we should run all tests (This is called a
"core/other modified" case. The assumption here is that modificiation
of any of the core code or some of the auxiliary utils might affect
everyone else.

However - we can safely assume that if only "tests/always" files
have been modified, then we can remove them from the list - because
those tests will anyhow will be executed and changing those tests
should have no impact on other tests. Those tests are ALWAYS executed.
iThis will - in some cases - avoid running full test suite when
we really only run a small subset of those.

This was the case for apache#35457 where it run full test suite, but really
only "Always Providers[common.io]" were needed.
@potiuk potiuk merged commit d21d56e into apache:main Nov 5, 2023
45 checks passed
@potiuk potiuk deleted the add-missing-common-io-test branch November 5, 2023 14:32
potiuk added a commit that referenced this pull request Nov 5, 2023
…5458)

When we decide which tests should be run we err on a safe side and
when we see that some files were modified that we cannot classify
to specific providers, API, CLI, WWW area, we err on a safe side
and assume that we should run all tests (This is called a
"core/other modified" case. The assumption here is that modificiation
of any of the core code or some of the auxiliary utils might affect
everyone else.

However - we can safely assume that if only "tests/always" files
have been modified, then we can remove them from the list - because
those tests will anyhow will be executed and changing those tests
should have no impact on other tests. Those tests are ALWAYS executed.
iThis will - in some cases - avoid running full test suite when
we really only run a small subset of those.

This was the case for #35457 where it run full test suite, but really
only "Always Providers[common.io]" were needed.
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Nov 10, 2023
Since our test to detect missing tests was broken, lack of the
test_file_transfer.py (among many others) has not been detected
before.

This PR adds a simple test that mocks out ObjectStoragePath and
tests that they are initialized properly and that execute method
executes `copy` on the source Object poth (but also does not call
copy on the target one).

Part of apache#35442

Also unblocks apache#35241 which fails becasuse Pytest fails when it sees
no tests collected when `Providers[common.io]'` TEST_TYPE is used.
romsharon98 pushed a commit to romsharon98/airflow that referenced this pull request Nov 10, 2023
…ache#35458)

When we decide which tests should be run we err on a safe side and
when we see that some files were modified that we cannot classify
to specific providers, API, CLI, WWW area, we err on a safe side
and assume that we should run all tests (This is called a
"core/other modified" case. The assumption here is that modificiation
of any of the core code or some of the auxiliary utils might affect
everyone else.

However - we can safely assume that if only "tests/always" files
have been modified, then we can remove them from the list - because
those tests will anyhow will be executed and changing those tests
should have no impact on other tests. Those tests are ALWAYS executed.
iThis will - in some cases - avoid running full test suite when
we really only run a small subset of those.

This was the case for apache#35457 where it run full test suite, but really
only "Always Providers[common.io]" were needed.
@ephraimbuddy ephraimbuddy added the changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..) label Nov 20, 2023
@ephraimbuddy ephraimbuddy added this to the Airflow 2.8.0 milestone Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:providers changelog:skip Changes that should be skipped from the changelog (CI, tests, etc..)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants