drill-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From paul-rogers <...@git.apache.org>
Subject [GitHub] drill pull request #984: DRILL-5783 Made a unit test for generated Priority ...
Date Wed, 18 Oct 2017 22:24:07 GMT
Github user paul-rogers commented on a diff in the pull request:

    https://github.com/apache/drill/pull/984#discussion_r145506779
  
    --- Diff: exec/java-exec/src/test/java/org/apache/drill/exec/physical/impl/writer/TestCorruptParquetDateCorrection.java
---
    @@ -377,21 +386,21 @@ public void testReadOldMetadataCacheFileOverrideCorrection() throws
Exception {
     
       @Test
       public void testReadNewMetadataCacheFileOverOldAndNewFiles() throws Exception {
    -    String table = format("dfs.`%s`", new Path(getDfsTestTmpSchemaLocation(), MIXED_CORRUPTED_AND_CORRECT_PARTITIONED_FOLDER));
    -    copyMetaDataCacheToTempReplacingInternalPaths(
    +    File meta = dirTestWatcher.copyResourceToRoot(
             "parquet/4203_corrupt_dates/mixed_version_partitioned_metadata.requires_replace.txt",
    -        MIXED_CORRUPTED_AND_CORRECT_PARTITIONED_FOLDER, Metadata.METADATA_FILENAME);
    +        Paths.get(MIXED_CORRUPTED_AND_CORRECT_PARTITIONED_FOLDER, Metadata.METADATA_FILENAME).toString());
    --- End diff --
    
    Seems awkward. We use a path to concatenate strings, then convert back to a string, then
to a file. Further, for some crazy reason, Hadoop also has a `Path` class that many tests
reference.
    
    Can we instead do:
    
    `File meta = ...toRoot(new File(new File(MIXED_CORRUPTED_AND_CORRECT_PARTITIONED_FOLDER),
Metadata.METADATA_FILENAME));
    ```
    
    Or, better, since seem to use this pattern over and over:
    
    ```
    File meta = ...toRoot(MIXED_CORRUPTED_AND_CORRECT_PARTITIONED_FOLDER, Metadata.METADATA_FILENAME);
    ```
    
    That is, the `copyResourceToRoot` has a form that takes a variable number of arguments
to build up the target path. Each should be assumed to be relative "a/b", "c", "d.csv", say.


---

Mime
View raw message