Integration tests for GCSFileSystem #28185
Conversation
|
Run Python 3.11 PostCommit |
1 similar comment
|
Run Python 3.11 PostCommit |
Codecov Report
@@ Coverage Diff @@
## master #28185 +/- ##
=======================================
Coverage 72.29% 72.30%
=======================================
Files 678 678
Lines 99848 99877 +29
=======================================
+ Hits 72189 72218 +29
Misses 26084 26084
Partials 1575 1575
Flags with carried forward coverage won't be shown. Click here to find out more. see 12 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
Run Python 3.11 PostCommit |
1 similar comment
|
Run Python 3.11 PostCommit |
BjornPrime
force-pushed
the
add-gcsfilesystem-it-test
branch
from
03d8ab8 to
e1de4e0
Compare
|
Run Python 3.11 PostCommit |
|
Assigning reviewers. If you would like to opt out of this review, comment R: @tvalentyn for label python. Available commands:
The PR bot will only process comments in the main thread (not review comments). |
| def setUp(self): | ||
| self.test_pipeline = TestPipeline(is_integration_test=True) | ||
| self.runner_name = type(self.test_pipeline.runner).__name__ | ||
| if self.runner_name != 'TestDataflowRunner': |
There was a problem hiding this comment.
Just a note - it's a little bit weird these tests need TestDataflowRunner (this is also set in current gcsio integration test). In the future good to reorganize tests to have a suite running on direct runner (local) and having gcp credentials.
There was a problem hiding this comment.
I will keep that in mind for the future. I was just following gcsio_integration_test's lead here.
| self._verify_rename(_old, _new) | ||
|
|
||
| @pytest.mark.it_postcommit | ||
| def test_rename_error(self): |
There was a problem hiding this comment.
It's good added a error handling test here. Still need to add batch_copy and batch_delete error handling tests which was the gap we currently have
There was a problem hiding this comment.
I added this because it's a gcsfilesystem method that exercises gcsio.copy_batch and gcsio.delete_batch and the identified gap is with how those methods communicate their results to gcsfilesystem.
There was a problem hiding this comment.
I see, yes currently rename() exercises both gcsio.copy_batch and gcsio.delete_batch.
Though adding gcsio_integration_tests are more direct, this also covers the code as of now.
| # Larger than 1MB to test maxBytesRewrittenPerCall. | ||
| # Also needs to be in a different region than the dest to take effect. | ||
| INPUT_FILE_LARGE = 'gs://apache-beam-samples-us-east1/wikipedia_edits/wiki_data-000000000000.json' # pylint: disable=line-too-long |
There was a problem hiding this comment.
minor clean up - this constant not used here
There was a problem hiding this comment.
Noted, will clean this up in migration PR.
| -Dtests=apache_beam.io.gcp.gcsfilesystem_integration_test:GcsFileSystemIntegrationTest # pylint: disable=line-too-long | ||
| """ | ||
|
|
||
| # pytype: skip-file |
There was a problem hiding this comment.
minor - lets enable linting for new files (some old py sources skipped linting because the check was introduced after they existed in repo)
Adding integration tests for GCSFileSystem to improve the odds of a clean GCS migration (see #25676).
Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:
addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, commentfixes #<ISSUE NUMBER>instead.CHANGES.mdwith noteworthy changes.See the Contributor Guide for more tips on how to make review process smoother.
To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md
GitHub Actions Tests Status (on master branch)
See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.