[GCSIO] Fix internal unit test failure #32518
Conversation
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
| @@ -1023,7 +1023,8 @@ def _handle_temp_and_staging_locations(self, validator): | |||
| def validate(self, validator): | |||
| errors = [] | |||
| if validator.is_service_runner(): | |||
| errors.extend(self._handle_temp_and_staging_locations(validator)) | |||
| if not self.view_as(TestOptions).dry_run: | |||
There was a problem hiding this comment.
Let's add a comment stating "this needs Internet connection and/or credentials"
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #32518 +/- ##
============================================
- Coverage 57.37% 57.36% -0.01%
Complexity 1474 1474
============================================
Files 966 966
Lines 153593 153595 +2
Branches 1076 1076
============================================
- Hits 88118 88114 -4
- Misses 63271 63277 +6
Partials 2204 2204
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
shunping
changed the title
|
@Abacn could you take another look? |
| @@ -718,6 +719,7 @@ def test_validation_good_stg_good_temp(self): | |||
| '--staging_location=gs://beam/stg', | |||
| '--temp_location=gs://beam/tmp' | |||
| ]) | |||
| options.view_as(TestOptions).dry_run = True | |||
There was a problem hiding this comment.
I tend to prefer mock as dryRun is too broad and may affect other behaviors that reduce test coverage
There was a problem hiding this comment.
Unfortunately, mock does not work when tests are run in our GitHub actions. Because we don't have google-cloud-storage installed for these unit tests on pipeline. As a result, mocking on 'apache_beam.io.gcp.gcsio' will give an error, and I don't want to have a condition inside the test to say "if we can load the package, we will do the mock; otherwise we skip it".
I am open to any suggestions though.
There was a problem hiding this comment.
An example of test failure if mocking is used https://github.com/apache/beam/actions/runs/10963819700/job/30446103406
There was a problem hiding this comment.
Actually, I have some idea which may need more changes. Will try and report back.
There was a problem hiding this comment.
Ok. I made some changes to mock conditionally. PTAL
* Fix internal unit test failure. * Minor refactor and add comment. * Fix test failure in github action. * Mock is_soft_delete_enabled only if gcsio can be loaded. * Disable unused import lint. It is using in mock. * Format
Skip or mock the check of
is_soft_delete_enabled, because it requires a networck connection to access GCS.