SDAP-161 MUDROD embedded unit test by quintinali · Pull Request #35 · apache/sdap-mudrod

quintinali · 2018-11-30T22:58:30Z

test case for log ingestion and preprocessing

asfgit · 2018-11-30T22:58:32Z

Can one of the admins verify this patch?

quintinali · 2018-11-30T23:02:37Z

@fgreg @lewismc Could you please take a look at the code. As I mentioned in the last meeting, the EmbeddedElasticsearchServer can not work in the code and I can not fix it. Could anyone help me to figure out what the problem is? If you compile the code, please use command "mvn clean install -Dskiptests", or the unit test will fail. I run these unit test cases with eclipses and a elasticsearch engine installed on my computer.

lewismc

@quintinali please please please remember. The source code is formatted at 2 space indents not 4. Please ensure that your code is formatted to 2 space indents.
Please see my comments... there is a lot we need to improve here before we can go forward.

lewismc · 2018-11-30T23:25:27Z

core/src/main/java/org/apache/sdap/mudrod/discoveryengine/WeblogDiscoveryEngine.java

      LOG.info("Processing logs dated {}", anInputList);

-      DiscoveryStepAbstract im = new ImportLogFile(this.props, this.es, this.spark);
+     /* DiscoveryStepAbstract im = new ImportLogFile(this.props, this.es, this.spark);


OK, the issue SDAP-161 is described as being MUDROD embedded unit test there should therefore be no code other than en embedded Unit test...

Additionally, if you are going to comment out code that you do not intend to use... just remove it.

lewismc · 2018-11-30T23:25:53Z

core/src/main/java/org/apache/sdap/mudrod/discoveryengine/WeblogDiscoveryEngine.java


-    DiscoveryStepAbstract hg = new HistoryGenerator(this.props, this.es, this.spark);
-    hg.execute();
+    /*DiscoveryStepAbstract hg = new HistoryGenerator(this.props, this.es, this.spark);


Same here...

lewismc · 2018-11-30T23:26:38Z

core/src/main/java/org/apache/sdap/mudrod/main/MudrodEngine.java

    } catch (Exception e) {
      HelpFormatter formatter = new HelpFormatter();
-      formatter.printHelp("MudrodEngine: 'dataDir' argument is mandatory. " + "User must also provide an ingest method.", true);
+      formatter.printHelp("MudrodEngine: 'dataDir' argument is mandatory. " + "User must also provide an ingest method.", options, true);


This code is buggy. We just want to throw the Exception... not continuously print the help arguments.

@quintinali please address this. It should not throw help. It should through the Exception e

@lewismc In my computer, the old code (
formatter.printHelp("MudrodEngine: 'dataDir' argument is mandatory. " + "User must also provide an ingest method.", true)) comes with an error “The method printHelp(String, Options) in the type HelpFormatter is not applicable for the arguments (String, boolean)”

OK what I am saying is that this code should be changed to the following

} catch (Exception e) { throw new RuntimeException(e) }

...or something similar.

core/src/main/java/org/apache/sdap/mudrod/main/MudrodEngine.java

core/src/main/java/org/apache/sdap/mudrod/weblog/pre/SessionStatistic.java

lewismc · 2018-11-30T23:36:53Z

core/src/test/java/org/apache/sdap/mudrod/weblog/structure/RequestUrlTest.java

+
+public class RequestUrlTest {
+
+//	@BeforeClass


Never leave code commented out like this it is extremely untidy.

lewismc · 2018-11-30T23:37:14Z

core/src/test/java/org/apache/sdap/mudrod/weblog/structure/RequestUrlTest.java

+	@Test
+	public void testuRLRequest() {
+		RequestUrl url = new RequestUrl();
+		String strURL = "https://podaac.jpl.nasa.gov/datasetlist?ids=Collections:Measurement:SpatialCoverage:Platform:Sensor&values=SCAT_BYU_L3_OW_SIGMA0_ENHANCED:Sea%20Ice:Bering%20Sea:ERS-2:AMI&view=list";


Why is this not a constant?

core/src/test/java/org/apache/sdap/mudrod/weblog/structure/log/GeoIpTest.java

lewismc · 2018-11-30T23:37:45Z

core/src/test/java/org/apache/sdap/mudrod/weblog/structure/log/GeoIpTest.java

@@ -0,0 +1,18 @@
+package org.apache.sdap.mudrod.weblog.structure.log;
+
+import static org.junit.Assert.*;


Never use wildcard imports.

lewismc · 2018-11-30T23:40:16Z

core/src/test/java/org/apache/sdap/mudrod/weblog/structure/log/GeoIpTest.java

+		Assert.assertEquals("failed in geoip function!", "22.283001,114.150002", result.latlon);
+	}
+
+}


We cannot ship binary code inside of source code management... the resources below should be decompressed.

Also, is there any reason we are using logs from 2015? Why not 2018?

I cannot understand which resource do you mean should be decompressed? There is not a specific reason for choosing logs from 2015.

core/src/test/resources/Testing_Data_1_3dayLog+Meta+Onto/FTP.201502.w1.gz and
core/src/test/resources/Testing_Data_1_3dayLog+Meta+Onto/WWW.201502.w1.gz

You have not addressed this issue. The files should not be in the compressed form... you need to decompress them.

I remembered that @fgreg has added a function of decompressing zip log file for MUDROD. And the test code can parse the two gz. file. If you need, I can depress them for you. Is it necessary?

1. formatter 2. remove useless import

quintinali · 2018-12-06T16:23:35Z

@lewismc I revised the code as you required. Could you please take a look at the unit test cases and please let me know whether they are the test case your team needs?

lewismc

@quintinali thanks for making some updates. Please see the remainder of my comments.

lewismc · 2018-12-06T17:12:38Z

core/src/main/java/org/apache/sdap/mudrod/main/MudrodEngine.java

    } catch (Exception e) {
      HelpFormatter formatter = new HelpFormatter();
-      formatter.printHelp("MudrodEngine: 'dataDir' argument is mandatory. " + "User must also provide an ingest method.", true);
+      formatter.printHelp("MudrodEngine: 'dataDir' argument is mandatory. " + "User must also provide an ingest method.", options, true);


@quintinali please address this. It should not throw help. It should through the Exception e

core/src/main/java/org/apache/sdap/mudrod/main/MudrodEngine.java

core/src/main/java/org/apache/sdap/mudrod/weblog/pre/SessionStatistic.java

lewismc · 2018-12-06T17:14:43Z

core/src/main/java/org/apache/sdap/mudrod/weblog/structure/session/SessionTree.java

      String viewquery = "";
      try {
-        String infoStr = requestURL.getSearchInfo(viewnode.getRequest());
+        //String infoStr = requestURL.getSearchInfo(viewnode.getRequest());


You need to resolve this.

lewismc · 2018-12-06T17:15:09Z

core/src/test/java/org/apache/sdap/mudrod/discoveryengine/WeblogDiscoveryEngineTest.java

+
+  @AfterClass
+  public static void tearDown() {
+    // TODO


Remove TODO and method if nothing happens here.

Resolve most issued metioned above except the first one. In my computer, the old code (
formatter.printHelp("MudrodEngine: 'dataDir' argument is mandatory. " + "User must also provide an ingest method.", true)) comes with an error “The method printHelp(String, Options) in the type HelpFormatter is not applicable for the arguments (String, boolean)”

lewismc · 2018-12-06T17:25:04Z

core/src/test/java/org/apache/sdap/mudrod/main/ESDriverTest.java

+  public void testDeleteAllByQuery() {
+    es.deleteAllByQuery("mudrod", "MetadataLinkage", QueryBuilders.matchAllQuery());
+
+    // String res = es.searchByQuery("mudrod", "MetadataLinkage",


lewismc · 2018-12-06T17:25:59Z

core/src/test/java/org/apache/sdap/mudrod/main/ESDriverTest.java

+  /*
+   * @Test public void testMakeClient() {
+   * 
+   * try { Client client = es.makeClient(mudrodEngine.loadConfig()); assert


Remove if not used. Why is it not being used?

lewismc · 2018-12-06T17:26:11Z

core/src/test/java/org/apache/sdap/mudrod/main/ESDriverTest.java

+  /*
+   * @Test public void testSetClient() {
+   * 
+   * try { Client client = es.makeClient(mudrodEngine.loadConfig());


Remove if not being used... why is it not being used?

lewismc · 2018-12-06T17:27:13Z

core/src/test/java/org/apache/sdap/mudrod/main/TestMudrodEngine.java

- * tests are implemented, this is merely in place to get the JaCoCo test reporting to
- * work.
+ * Initial test case for {@link org.apache.sdap.mudrod.main.MudrodEngine},
+ * currently no tests are implemented, this is merely in place to get the JaCoCo


If no tests are implemented then this is a TODO and a JIRA issue should be logged such that someone knows that a test needs to be written.

I delete the test class for MudrodEngine and will add it in the future

lewismc · 2018-12-06T17:28:31Z

core/src/test/java/org/apache/sdap/mudrod/weblog/structure/log/GeoIpTest.java

+		Assert.assertEquals("failed in geoip function!", "22.283001,114.150002", result.latlon);
+	}
+
+}


core/src/test/resources/Testing_Data_1_3dayLog+Meta+Onto/FTP.201502.w1.gz and
core/src/test/resources/Testing_Data_1_3dayLog+Meta+Onto/WWW.201502.w1.gz

quintinali · 2018-12-07T20:49:15Z

@lewismc I solved most issued based on your comments. Could you please take a look at my explanation when you have time, especially for this one #35 (comment)

Besides, sorry that I missed some comments before since some conversations were displayed in a hidden mode and I didn't notice that.

lewismc

Thanks for update @quintinali please see my new comments.

lewismc · 2018-12-08T20:21:51Z

core/src/main/java/org/apache/sdap/mudrod/main/MudrodEngine.java

    } catch (Exception e) {
      HelpFormatter formatter = new HelpFormatter();
-      formatter.printHelp("MudrodEngine: 'dataDir' argument is mandatory. " + "User must also provide an ingest method.", true);
+      formatter.printHelp("MudrodEngine: 'dataDir' argument is mandatory. " + "User must also provide an ingest method.", options, true);


OK what I am saying is that this code should be changed to the following

} catch (Exception e) { throw new RuntimeException(e) }

...or something similar.

lewismc · 2018-12-08T20:23:30Z

core/src/test/java/org/apache/sdap/mudrod/discoveryengine/WeblogDiscoveryEngineTest.java

+  }
+
+  private static String getTestDataPath() {
+    File resourcesDirectory = new File("src/test/resources/");


This still needs to be addressed using the ClassLoader. Something like

getClass().getClassLoader().getResource... or getResourceAsStream...

You need to make this change rather than passing around File objects OK.

lewismc · 2018-12-08T20:24:40Z

core/src/test/java/org/apache/sdap/mudrod/driver/EmbeddedElasticsearchServer.java

+    return node.client();
+  }
+
+  public void shutdown() {


Remove this method if it has no content and you are not overriding anything.

lewismc · 2018-12-08T20:25:33Z

core/src/test/java/org/apache/sdap/mudrod/main/AbstractElasticsearchIntegrationTest.java

+import org.junit.BeforeClass;
+
+/**
+ * This is a helper class the starts an embedded elasticsearch server for each


Once you have addressed the comments here fully I will test the code locally and provide input as to why it is not working.

lewismc · 2018-12-08T20:26:09Z

core/src/test/java/org/apache/sdap/mudrod/main/ESDriverTest.java

+
+  @Test
+  public void testCustomAnalyzingStringString() {
+


Remove whitespace

lewismc · 2018-12-08T20:26:49Z

core/src/test/java/org/apache/sdap/mudrod/main/ESDriverTest.java

+
+  @Test
+  public void testGenerateUpdateRequestStringStringStringMapOfStringObject() {
+


Remove whitespace

lewismc · 2018-12-08T20:26:53Z

core/src/test/java/org/apache/sdap/mudrod/main/ESDriverTest.java

+
+  @Test
+  public void testGetDocCountStringStringArray() {
+


Remove whitespace

lewismc · 2018-12-08T20:26:57Z

core/src/test/java/org/apache/sdap/mudrod/main/ESDriverTest.java

+
+  @Test
+  public void testGetDocCountStringArrayStringArray() {
+


Remove whitespace

lewismc · 2018-12-08T20:27:00Z

core/src/test/java/org/apache/sdap/mudrod/main/ESDriverTest.java

+
+  @Test
+  public void testGetDocCountStringArrayStringArrayQueryBuilder() {
+


Remove whitespace

lewismc · 2018-12-08T20:27:48Z

core/src/test/java/org/apache/sdap/mudrod/weblog/structure/log/GeoIpTest.java

+		Assert.assertEquals("failed in geoip function!", "22.283001,114.150002", result.latlon);
+	}
+
+}


You have not addressed this issue. The files should not be in the compressed form... you need to decompress them.

quintinali · 2018-12-10T17:03:07Z

@lewismc I revised code based on your comments. Please take a look at the embedded elasticsearch engine when you have time. Thanks.

For the compressed logs, MUDROD has the capability to decompress them. Please check the attached figure.

Fitting bugs

# Conflicts: # core/src/main/java/org/apache/sdap/mudrod/main/MudrodEngine.java # core/src/test/java/org/apache/sdap/mudrod/weblog/structure/TestApacheAccessLog.java

quintinali added 6 commits June 29, 2018 09:55

Merge remote-tracking branch 'refs/remotes/apache/master'

70e84a1

Merge remote-tracking branch 'refs/remotes/apache/master'

9145156

add embedded elasticsearch engine

d0e1345

log preprocessing test case

70d1fbf

Merge remote-tracking branch 'refs/remotes/apache/master' into SDAP-161

e8a9fce

test data for log preprocessing

b3ef470

lewismc changed the title ~~Sdap 161~~ SDAP-161 MUDROD embedded unit test Nov 30, 2018

lewismc requested changes Nov 30, 2018

View reviewed changes

modify code as request

08337c1

1. formatter 2. remove useless import

lewismc requested changes Dec 6, 2018

View reviewed changes

Solved most comments provided by the reviewer

8b28adc

lewismc requested changes Dec 8, 2018

View reviewed changes

Revised code based on Lewis's comments

122b228

quintinali and others added 6 commits January 15, 2019 10:29

solved the embedded elasticsearch engine issue

04c9cf3

Merge remote-tracking branch 'refs/remotes/apache/master'

73e1cee

Merge remote-tracking branch 'refs/remotes/apache/master'

af2a3d9

Fitting Bugs

024d545

Fitting bugs

Merge remote-tracking branch 'refs/remotes/origin/master' into SDAP-161

5d57792

# Conflicts: # core/src/main/java/org/apache/sdap/mudrod/main/MudrodEngine.java # core/src/test/java/org/apache/sdap/mudrod/weblog/structure/TestApacheAccessLog.java

Fix bugs

da22f9c

		@@ -0,0 +1,18 @@
		package org.apache.sdap.mudrod.weblog.structure.log;

		import static org.junit.Assert.*;


		@Test
		public void testGenerateUpdateRequestStringStringStringMapOfStringObject() {


		@Test
		public void testGetDocCountStringArrayStringArrayQueryBuilder() {

Conversation

quintinali commented Nov 30, 2018

Uh oh!

asfgit commented Nov 30, 2018

Uh oh!

quintinali commented Nov 30, 2018

Uh oh!

lewismc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

quintinali commented Dec 6, 2018

Uh oh!

lewismc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

quintinali commented Dec 7, 2018

Uh oh!

lewismc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment