[ 
https://issues.apache.org/jira/browse/BIGTOP-1222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14076931#comment-14076931
 ] 

Konstantin Boudnik commented on BIGTOP-1222:
--------------------------------------------

- inlined versions
bq. +    testCompile group: 'org.apache.bigtop.itest', name: 'itest-common', 
version: '0.7.0', transitive: 'true'
Shall it be at 0.8.0, no? Also, would it make sense to have these versions 
defined somewhere at the top, instead of being hard-coded?
- also it seems that dependencies section is being propagated through all the 
build files and the versions are being inlined everywhere. Can it be defined at 
the top-level build.gradle and reused downstream?
- I see some weird stuff with the same files having different content and 
different paths. Like
{code}
diff --git a/bigtop-smoke-tests/flume/conf/flume.conf 
b/bigtop-smoke-tests/flume/conf/flume.conf
diff --git a/bigtop-smoke-tests/flume/flume.conf 
b/bigtop-smoke-tests/flume/flume.conf
{code}
What's the purpose of it?
- do you really need empty lines in the Runtime exception throw like
{code}
+    throw new RuntimeException(
+            """
+
+      Oops! You forgot to define some tests!
{code}
- I don't like the fact that we need to manually list the tests in the 
build.gradle file. The case in point
{code}
+def tests_to_include() {
+    return [
+            "TestSqoopETLHsql.groovy"
+    ];
+}
{code}
Can't it be done dynamically?
- is the wording change relevant to the scope of the JIRA
{code}
-        assertTrue("Could not create input directory to HDFS", sh.getRet() == 
0);
+        assertTrue("Could not create input directory to the DFS", sh.getRet() 
== 0);
{code}
or has it been done because "we were at it anyway". Can we keep only relevant 
changes so it is easy to track them later?
- shall the logic inside of the "catch ... Throwable" (class 
TestHadoopExamples) be moved into {{TestUtils.unpackTestResources}} or a new 
one? Do you think this is a common retry pattern that will be commonly used 
elsewhere?
- is it a relevant change? Looks like it changes the defaults:
{code}
-  public static String pi_samples = System.getProperty("pi_samples", "1000");
-
+  public static String pi_samples = System.getProperty("pi_samples", "2");
{code}
- if this code isn't used anymore - just remove it
{code}
-    TestUtils.unpackTestResources(TestHadoopSmoke.class, 
"${testDir}/cachefile", inputFiles, null);
+    //TestUtils.unpackTestResources(TestHadoopSmoke.class, 
"${testDir}/cachefile", inputFiles, null);
{code}

The indentation varies widely, so please fix it as well.
Also there are some occasional doubled empty-lines, which makes sense to trim 
off.

Great job overall! Thank you!

> Simplify and gradleize a subset of the bigtop smokes
> ----------------------------------------------------
>
>                 Key: BIGTOP-1222
>                 URL: https://issues.apache.org/jira/browse/BIGTOP-1222
>             Project: Bigtop
>          Issue Type: Improvement
>          Components: Build, Tests
>    Affects Versions: 0.7.0
>            Reporter: jay vyas
>            Assignee: Konstantin Boudnik
>             Fix For: backlog
>
>         Attachments: BIGTOP-1222-2.patch, BIGTOP-1222.patch, 
> BIGTOP-1222.patch, BIGTOP-1222.patch, BIGTOP-1222.patch, BIGTOP-1222.patch, 
> BIGTOP-1222.patch, BIGTOP-1222.patch, BIGTOP-1222.patch, BIGTOP-1222.patch, 
> BIGTOP-1222.patch
>
>
> (Rewritten the description for clarity)
> We need an easier way to run bigtop smoke tests, and gradle provides this:
> 1) Easy to script/modify
> 2) Human readable
> 3) equally oriented towards both groovy and plain old java
> The advantage of this method to running smokes : 
> 1) No need to compile a jar : this is a costly step and not much value added, 
> also creates indirection which can make debugging a broken test very hard.
> 2) Simple: A smoke test doesnt need to make low level API calls or be 
> compiled against the right APIs - rather, it should test the end user 
> interface ("hive -q  ....", "pig -x ....", "hadoop jar ....", and so on).  
> 3) Customizable:  The smoke tests shouldnt require users to have to write XML 
> and debug environmental variables / grep around for System properties etc.  
> Rather, a high level controller should do all that checking for you.  
> The initial idea was to write a python/bash implementation wrapper of 
> scripts, but that was replaced by the idea of using gradle.  The advantage of 
> gradle is that we don't need to manually set the classpath and run groovy 
> commands: Gradle wraps groovy scripts in their native java context quite 
> nicely - but it doesnt add any other unnecessary overhead (xml, jar files, no 
> need for complex xml tag wrappers for simple tasks - just plain groovy code).
> So, here the goal is just to create a nice, clean, extensible non-jar, 
> non-API dependent gradle runner for the smoke tests which exersizes the 
> hadoop cluster the same way a typical end-user would.



--
This message was sent by Atlassian JIRA
(v6.2#6252)

Reply via email to