[GitHub] [gobblin] umustafi commented on a change in pull request #3459: [GOBBLIN-1602] Change hive table location and partition check to validate using FS r…

[GitBox <gi...@apache.org>]

umustafi commented on a change in pull request #3459:
URL: https://github.com/apache/gobblin/pull/3459#discussion_r799926493



##########
File path: gobblin-data-management/src/test/java/org/apache/gobblin/data/management/copy/hive/UnpartitionedTableFileSetTest.java
##########
@@ -44,11 +48,47 @@ public void testHiveTableLocationNotMatchException() throws Exception {
         Mockito.when(helper.getDataset()).thenReturn(hiveDataset);
         Mockito.when(helper.getExistingTargetTable()).thenReturn(Optional.of(existingTargetTable));
         Mockito.when(helper.getTargetTable()).thenReturn(table);
+        // Mock filesystem resolver
+        FileSystem mockFS = Mockito.mock(FileSystem.class);
+        Mockito.when(helper.getTargetFs()).thenReturn(mockFS);
+        Mockito.when(mockFS.resolvePath(Mockito.any())).then(returnsFirstArg());
+
+        Mockito.when(helper.getExistingEntityPolicy()).thenReturn(HiveCopyEntityHelper.ExistingEntityPolicy.ABORT);
+        MetricContext metricContext = MetricContext.builder("testUnpartitionedTableFileSet").build();
+        EventSubmitter eventSubmitter = new EventSubmitter.Builder(metricContext,"loc.nomatch.exp").build();
+        Mockito.when(helper.getEventSubmitter()).thenReturn(eventSubmitter);
+        UnpartitionedTableFileSet upts = new UnpartitionedTableFileSet("testLocationMatch",hiveDataset,helper);
+        List<CopyEntity> copyEntities = (List<CopyEntity>)upts.generateCopyEntities();
+    }
+
+    @Test
+    public void testHiveTableLocationMatchDifferentPathsResolved() throws Exception {
+        Path testPath = new Path("/testPath/db/table");
+        Path existingTablePath = new Path("/existing/testPath/db/table");

Review comment:
       Is it worth adding a test case comparing logical and physical paths to be explicit about what we're testing the difference between? If existing is logical and user specified is physical, then we don't want it to use the physical path right? 
   
   Also why are the partitioned and snapshot cases different? For partitioned cases if paths differ we throw and error and abort, but for snapshot we can check the policy and overwrite location as needed. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org