You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ma...@apache.org on 2020/05/12 16:28:27 UTC

[incubator-pinot] branch master updated: Fix in PluginManagerTest to fail if TestRecordReader cannot be compiled. (#5373)

This is an automated email from the ASF dual-hosted git repository.

mayanks pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git


The following commit(s) were added to refs/heads/master by this push:
     new 84c9d5c  Fix in PluginManagerTest to fail if TestRecordReader cannot be compiled. (#5373)
84c9d5c is described below

commit 84c9d5c09acdb0c85f8632bba99b888c33fdb9fa
Author: Mayank Shrivastava <ms...@linkedin.com>
AuthorDate: Tue May 12 09:28:17 2020 -0700

    Fix in PluginManagerTest to fail if TestRecordReader cannot be compiled. (#5373)
    
    With a recent change in RecordReader interface, two issues where exposed:
    
    1. TestRecordReader.java that implements the interface was not updated as it is under
       the resources directory, so it is not compiled (and hence does not lead to build failures).
    
    2. It is read in as a resource inside of PluginManagerTest that compiles the file, which should
       have caught the compile issue, however, the test simply ignores the compile error and ends
       up passing (incorrectly).
    
    This PR fixes the issue in PluginManagerTest to ensure that compile errors in TestRecordReader.java
    are caught when the test is run, thus solving both problems.
---
 .../test/java/org/apache/pinot/spi/plugin/PluginManagerTest.java | 7 +++++--
 pinot-spi/src/test/resources/TestRecordReader.java               | 9 ++-------
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/pinot-spi/src/test/java/org/apache/pinot/spi/plugin/PluginManagerTest.java b/pinot-spi/src/test/java/org/apache/pinot/spi/plugin/PluginManagerTest.java
index 0dfd4ee..ac0f170 100644
--- a/pinot-spi/src/test/java/org/apache/pinot/spi/plugin/PluginManagerTest.java
+++ b/pinot-spi/src/test/java/org/apache/pinot/spi/plugin/PluginManagerTest.java
@@ -36,6 +36,8 @@ import org.testng.annotations.Test;
 
 public class PluginManagerTest {
 
+  private final String TEST_RECORD_READER_FILE = "TestRecordReader.java";
+
   private File tempDir;
   private String jarFile;
   private File jarDirFile;
@@ -57,9 +59,10 @@ public class PluginManagerTest {
   public void testSimple()
       throws Exception {
     JavaCompiler compiler = ToolProvider.getSystemJavaCompiler();
-    URL javaFile = Thread.currentThread().getContextClassLoader().getResource("TestRecordReader.java");
+    URL javaFile = Thread.currentThread().getContextClassLoader().getResource(TEST_RECORD_READER_FILE);
     if (javaFile != null) {
-      compiler.run(null, null, null, javaFile.getFile(), "-d", tempDir.getAbsolutePath());
+      int compileStatus = compiler.run(null, null, null, javaFile.getFile(), "-d", tempDir.getAbsolutePath());
+      Assert.assertTrue(compileStatus == 0, "Error when compiling resource: " + TEST_RECORD_READER_FILE);
 
       URL classFile = Thread.currentThread().getContextClassLoader().getResource("TestRecordReader.class");
 
diff --git a/pinot-spi/src/test/resources/TestRecordReader.java b/pinot-spi/src/test/resources/TestRecordReader.java
index a223d58..9cf34db 100644
--- a/pinot-spi/src/test/resources/TestRecordReader.java
+++ b/pinot-spi/src/test/resources/TestRecordReader.java
@@ -20,11 +20,9 @@ public class TestRecordReader implements RecordReader {
 
   List<GenericRow> _rows = new ArrayList<>();
   Iterator<GenericRow> _iterator;
-  Schema _schema;
 
-  public void init(File dataFile, Schema schema, @Nullable RecordReaderConfig recordReaderConfig, Set<String> fields)
+  public void init(File dataFile, Set<String> fields, @Nullable RecordReaderConfig recordReaderConfig)
       throws IOException {
-    _schema = schema;
     int numRows = 10;
     for (int i = 0; i < numRows; i++) {
       GenericRow row = new GenericRow();
@@ -53,10 +51,7 @@ public class TestRecordReader implements RecordReader {
     _iterator = _rows.iterator();
   }
 
-  public Schema getSchema() {
-    return _schema;
-  }
-  public void close () {
+  public void close() {
 
   }
 }
\ No newline at end of file


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org