You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2020/06/10 13:44:26 UTC

[GitHub] [hadoop-ozone] elek opened a new pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

elek opened a new pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050


   ## What changes were proposed in this pull request?
   
   This patch adds the coverage data from the acceptance test to the generic coverage measurement.
   
   There was one question during the implementation: I decided to add the required HADOOP_OPTS to all the docker-compose file without using tricky docker-compose extension. I found that I need to add a few lines anyway, and I preferred to keep it simple, even if a possible change would require slightly more work (but can be done with easy search and replace)
   
   ## What is the link to the Apache JIRA
   
   https://issues.apache.org/jira/browse/HDDS-3757
   
   ## How was this patch tested?
   
   Pushed the branch to apache repo and checked sonar cloud.
   
   https://sonarcloud.io/dashboard?branch=HDDS-3757&id=hadoop-ozone


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] codecov-commenter commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-643631707


   # [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=h1) Report
   > Merging [#1050](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hadoop-ozone/commit/bd88eec9fec06a8f8adcbe4d257c5ccaca144f4c&el=desc) will **increase** coverage by `0.04%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/graphs/tree.svg?width=650&height=150&src=pr&token=5YeeptJMby)](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1050      +/-   ##
   ============================================
   + Coverage     69.36%   69.41%   +0.04%     
   - Complexity     9097     9109      +12     
   ============================================
     Files           961      961              
     Lines         48127    48127              
     Branches       4676     4676              
   ============================================
   + Hits          33383    33407      +24     
   + Misses        12524    12507      -17     
   + Partials       2220     2213       -7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...hdds/scm/container/common/helpers/ExcludeList.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vY29udGFpbmVyL2NvbW1vbi9oZWxwZXJzL0V4Y2x1ZGVMaXN0LmphdmE=) | `83.67% <0.00%> (-2.05%)` | `20.00% <0.00%> (-1.00%)` | |
   | [...apache/hadoop/ozone/client/io/KeyOutputStream.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL2NsaWVudC9pby9LZXlPdXRwdXRTdHJlYW0uamF2YQ==) | `79.58% <0.00%> (-0.84%)` | `46.00% <0.00%> (-1.00%)` | |
   | [...hadoop/ozone/om/ratis/OzoneManagerRatisServer.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9yYXRpcy9Pem9uZU1hbmFnZXJSYXRpc1NlcnZlci5qYXZh) | `79.29% <0.00%> (-0.79%)` | `35.00% <0.00%> (-1.00%)` | |
   | [...doop/ozone/container/keyvalue/KeyValueHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIva2V5dmFsdWUvS2V5VmFsdWVIYW5kbGVyLmphdmE=) | `61.11% <0.00%> (+0.22%)` | `62.00% <0.00%> (+1.00%)` | |
   | [.../transport/server/ratis/ContainerStateMachine.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3RyYW5zcG9ydC9zZXJ2ZXIvcmF0aXMvQ29udGFpbmVyU3RhdGVNYWNoaW5lLmphdmE=) | `69.36% <0.00%> (+0.22%)` | `59.00% <0.00%> (+1.00%)` | |
   | [.../apache/hadoop/ozone/om/OmMetadataManagerImpl.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9PbU1ldGFkYXRhTWFuYWdlckltcGwuamF2YQ==) | `82.52% <0.00%> (+0.26%)` | `92.00% <0.00%> (+1.00%)` | |
   | [.../ozone/container/common/volume/AbstractFuture.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3ZvbHVtZS9BYnN0cmFjdEZ1dHVyZS5qYXZh) | `30.64% <0.00%> (+0.77%)` | `21.00% <0.00%> (+2.00%)` | |
   | [...che/hadoop/hdds/scm/block/DeletedBlockLogImpl.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL2Jsb2NrL0RlbGV0ZWRCbG9ja0xvZ0ltcGwuamF2YQ==) | `72.60% <0.00%> (+1.36%)` | `22.00% <0.00%> (+1.00%)` | |
   | [.../apache/hadoop/ozone/client/io/KeyInputStream.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL2NsaWVudC9pby9LZXlJbnB1dFN0cmVhbS5qYXZh) | `79.69% <0.00%> (+1.50%)` | `43.00% <0.00%> (+1.00%)` | |
   | [...ine/commandhandler/DeleteBlocksCommandHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3N0YXRlbWFjaGluZS9jb21tYW5kaGFuZGxlci9EZWxldGVCbG9ja3NDb21tYW5kSGFuZGxlci5qYXZh) | `64.06% <0.00%> (+1.56%)` | `11.00% <0.00%> (+2.00%)` | |
   | ... and [6 more](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=footer). Last update [bd88eec...b833fef](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r438590181



##########
File path: hadoop-ozone/dev-support/checks/acceptance.sh
##########
@@ -17,19 +17,32 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 cd "$DIR/../../.." || exit 1
 
 REPORT_DIR=${OUTPUT_DIR:-"$DIR/../../../target/acceptance"}
-mkdir -p "$REPORT_DIR"
 
 OZONE_VERSION=$(grep "<ozone.version>" "pom.xml" | sed 's/<[^>]*>//g'|  sed 's/^[ \t]*//')
 DIST_DIR="$DIR/../../dist/target/ozone-$OZONE_VERSION"
 
 if [ ! -d "$DIST_DIR" ]; then
     echo "Distribution dir is missing. Doing a full build"
-    "$DIR/build.sh"
+    "$DIR/build.sh" -Pjacoco
 fi
 
+mkdir -p "$REPORT_DIR"
+
+export HADOOP_OPTS='-javaagent:/opt/hadoop/share/jacoco/jacoco-agent.jar=destfile=/tmp/jacoco.exec,includes=org.apache.hadoop.ozone.*:org.apache.hadoop.hdds'

Review comment:
       Now `acceptance.sh` requires `jacoco-agent.jar`, which is only built when invoked with `-Pjacoco`.  I would like to suggest moving this definition to the GitHub Actions definition (in the `env` section, where `KEEP_IMAGE` is defined, too).

##########
File path: hadoop-ozone/dist/src/main/compose/ozone-csi/docker-compose.yaml
##########
@@ -23,6 +23,8 @@ services:
       - ../..:/opt/hadoop
     env_file:
       - docker-config
+    environment:
+      HADOOP_OPTS: ${HADOOP_OPTS}

Review comment:
       I think we can avoid these by passing the variable via `-e` option of `docker-compose`, similar to how it is done for some other variables (`SECURITY_ENABLED`, etc.):
   
   https://github.com/apache/hadoop-ozone/blob/53395a0ed75d96575a7dc26a7adec4eefabb2b74/hadoop-ozone/dist/src/main/compose/testlib.sh#L107




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r445652086



##########
File path: hadoop-ozone/dist/src/main/compose/test-all.sh
##########
@@ -19,17 +19,22 @@
 #
 # Test executor to test all the compose/*/test.sh test scripts.
 #
-
 SCRIPT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )
 ALL_RESULT_DIR="$SCRIPT_DIR/result"
-
+PROJECT_DIR="$SCRIPT_DIR/.."
 mkdir -p "$ALL_RESULT_DIR"
-rm "$ALL_RESULT_DIR/*"
+rm "$ALL_RESULT_DIR/*" || true
+
+if [ "$OZONE_WITH_COVERAGE" ]; then
+   java -cp "$PROJECT_DIR"/share/coverage/$(ls "$PROJECT_DIR"/share/coverage | grep test-util):"$PROJECT_DIR"/share/coverage/jacoco-core.jar org.apache.hadoop.test.JacocoServer &
+   DOCKER_BRIDGE_IP=$(docker network inspect bridge --format='{{(index .IPAM.Config 0).Gateway}}')

Review comment:
       IP is available, can be pinged, but connection to port is refused.  Confirmed that Jacoco server is running on localhost:6300.
   
   I'm [experimenting](https://github.com/adoroszlai/hadoop-ozone/commits/HDDS-3757) with running Jacoco in its own container.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r446304146



##########
File path: hadoop-hdds/test-utils/src/main/java/org/apache/hadoop/test/JacocoServer.java
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.test;
+
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.net.ServerSocket;
+import java.net.Socket;
+
+import org.jacoco.core.data.ExecutionDataWriter;
+import org.jacoco.core.runtime.RemoteControlReader;
+import org.jacoco.core.runtime.RemoteControlWriter;
+
+/**
+ * Simple TPC server to collect all the Jacoco coverage data.
+ */
+public final class JacocoServer {
+
+  private static int port = 6300;
+
+  private static String destinationFile = "/tmp/jacoco-combined.exec";
+
+  private JacocoServer() {
+  }
+
+  @SuppressWarnings("checkstyle:EmptyStatement")
+  public static void main(String[] args) throws IOException {
+    ExecutionDataWriter destination =
+        new ExecutionDataWriter(new FileOutputStream(destinationFile));
+    ServerSocket serverSocket = new ServerSocket(port);
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+      try {
+        destination.flush();
+        serverSocket.close();
+      } catch (Exception ex) {
+        ex.printStackTrace();
+      }
+    }));
+
+    while (true) {
+      final Socket socket = serverSocket.accept();
+      new Thread(() -> {
+        try {
+          RemoteControlWriter writer =
+              new RemoteControlWriter(socket.getOutputStream());
+          RemoteControlReader reader =
+              new RemoteControlReader(socket.getInputStream());
+          reader.setSessionInfoVisitor(destination::visitSessionInfo);
+          reader.setExecutionDataVisitor(destination::visitClassExecution);
+          while (reader.read()) {
+            ;//read until the end of the stream.
+          }
+          destination.flush();

Review comment:
       added.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-650287131


   Finally, acceptance tests seem to be stable.


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r445367714



##########
File path: hadoop-ozone/dist/src/main/compose/test-all.sh
##########
@@ -41,8 +46,13 @@ for test in $(find "$SCRIPT_DIR" -name test.sh | sort); do
       echo "ERROR: Test execution of $(dirname "$test") is FAILED!!!!"
   fi
   RESULT_DIR="$(dirname "$test")/result"
-  cp "$RESULT_DIR"/robot-*.xml "$RESULT_DIR"/docker-*.log "$RESULT_DIR"/*.out* "$ALL_RESULT_DIR"/
+  cp "$RESULT_DIR"/robot-*.xml "$RESULT_DIR"/docker-*.log "$RESULT_DIR"/*.out* "$RESULT_DIR"/*.exec "$ALL_RESULT_DIR"/
 done
 
 rebot -N "smoketests" -d "$SCRIPT_DIR/result" "$SCRIPT_DIR/result/robot-*.xml"
+if [ "$OZONE_WITH_COVERAGE" ]; then
+  cp /tmp/jacoco-combined.exec "$SCRIPT_DIR/result/"

Review comment:
       Should work now.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] codecov-commenter edited a comment on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-643631707


   # [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=h1) Report
   > Merging [#1050](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hadoop-ozone/commit/57a8388b902a291571bdf60ab3f91930a5cf8815&el=desc) will **decrease** coverage by `0.13%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/graphs/tree.svg?width=650&height=150&src=pr&token=5YeeptJMby)](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1050      +/-   ##
   ============================================
   - Coverage     70.46%   70.33%   -0.14%     
   + Complexity     9242     9227      -15     
   ============================================
     Files           961      961              
     Lines         48130    48130              
     Branches       4676     4676              
   ============================================
   - Hits          33917    33852      -65     
   - Misses        11960    12021      +61     
   - Partials       2253     2257       +4     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...otocol/commands/RetriableDatanodeEventWatcher.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL3Byb3RvY29sL2NvbW1hbmRzL1JldHJpYWJsZURhdGFub2RlRXZlbnRXYXRjaGVyLmphdmE=) | `55.55% <0.00%> (-44.45%)` | `3.00% <0.00%> (-1.00%)` | |
   | [...hdds/scm/container/CloseContainerEventHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL2NvbnRhaW5lci9DbG9zZUNvbnRhaW5lckV2ZW50SGFuZGxlci5qYXZh) | `72.41% <0.00%> (-17.25%)` | `6.00% <0.00%> (ø%)` | |
   | [.../apache/hadoop/hdds/scm/node/StaleNodeHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL25vZGUvU3RhbGVOb2RlSGFuZGxlci5qYXZh) | `88.88% <0.00%> (-11.12%)` | `4.00% <0.00%> (ø%)` | |
   | [...hdds/scm/container/common/helpers/ExcludeList.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vY29udGFpbmVyL2NvbW1vbi9oZWxwZXJzL0V4Y2x1ZGVMaXN0LmphdmE=) | `75.51% <0.00%> (-8.17%)` | `18.00% <0.00%> (-2.00%)` | |
   | [.../transport/server/ratis/ContainerStateMachine.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3RyYW5zcG9ydC9zZXJ2ZXIvcmF0aXMvQ29udGFpbmVyU3RhdGVNYWNoaW5lLmphdmE=) | `69.36% <0.00%> (-6.76%)` | `59.00% <0.00%> (-5.00%)` | |
   | [...ozone/container/ozoneimpl/ContainerController.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvb3pvbmVpbXBsL0NvbnRhaW5lckNvbnRyb2xsZXIuamF2YQ==) | `63.15% <0.00%> (-5.27%)` | `11.00% <0.00%> (-1.00%)` | |
   | [...e/commandhandler/CreatePipelineCommandHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3N0YXRlbWFjaGluZS9jb21tYW5kaGFuZGxlci9DcmVhdGVQaXBlbGluZUNvbW1hbmRIYW5kbGVyLmphdmE=) | `87.23% <0.00%> (-4.26%)` | `8.00% <0.00%> (ø%)` | |
   | [...apache/hadoop/hdds/server/events/EventWatcher.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvZnJhbWV3b3JrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zZXJ2ZXIvZXZlbnRzL0V2ZW50V2F0Y2hlci5qYXZh) | `77.77% <0.00%> (-4.17%)` | `14.00% <0.00%> (ø%)` | |
   | [...iner/common/transport/server/ratis/CSMMetrics.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3RyYW5zcG9ydC9zZXJ2ZXIvcmF0aXMvQ1NNTWV0cmljcy5qYXZh) | `67.69% <0.00%> (-3.08%)` | `19.00% <0.00%> (-1.00%)` | |
   | [...ent/algorithms/SCMContainerPlacementRackAware.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL2NvbnRhaW5lci9wbGFjZW1lbnQvYWxnb3JpdGhtcy9TQ01Db250YWluZXJQbGFjZW1lbnRSYWNrQXdhcmUuamF2YQ==) | `76.69% <0.00%> (-3.01%)` | `31.00% <0.00%> (-2.00%)` | |
   | ... and [13 more](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=footer). Last update [57a8388...569cabb](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] codecov-commenter edited a comment on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-643631707


   # [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=h1) Report
   > Merging [#1050](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hadoop-ozone/commit/57a8388b902a291571bdf60ab3f91930a5cf8815&el=desc) will **decrease** coverage by `0.06%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/graphs/tree.svg?width=650&height=150&src=pr&token=5YeeptJMby)](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1050      +/-   ##
   ============================================
   - Coverage     70.46%   70.40%   -0.07%     
   + Complexity     9242     9237       -5     
   ============================================
     Files           961      961              
     Lines         48130    48130              
     Branches       4676     4676              
   ============================================
   - Hits          33917    33886      -31     
   - Misses        11960    11990      +30     
   - Partials       2253     2254       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...hdds/scm/container/CloseContainerEventHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL2NvbnRhaW5lci9DbG9zZUNvbnRhaW5lckV2ZW50SGFuZGxlci5qYXZh) | `72.41% <0.00%> (-17.25%)` | `6.00% <0.00%> (ø%)` | |
   | [.../transport/server/ratis/ContainerStateMachine.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3RyYW5zcG9ydC9zZXJ2ZXIvcmF0aXMvQ29udGFpbmVyU3RhdGVNYWNoaW5lLmphdmE=) | `69.81% <0.00%> (-6.31%)` | `60.00% <0.00%> (-4.00%)` | |
   | [...ozone/container/ozoneimpl/ContainerController.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvb3pvbmVpbXBsL0NvbnRhaW5lckNvbnRyb2xsZXIuamF2YQ==) | `63.15% <0.00%> (-5.27%)` | `11.00% <0.00%> (-1.00%)` | |
   | [...iner/common/transport/server/ratis/CSMMetrics.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3RyYW5zcG9ydC9zZXJ2ZXIvcmF0aXMvQ1NNTWV0cmljcy5qYXZh) | `67.69% <0.00%> (-3.08%)` | `19.00% <0.00%> (-1.00%)` | |
   | [...ent/algorithms/SCMContainerPlacementRackAware.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL2NvbnRhaW5lci9wbGFjZW1lbnQvYWxnb3JpdGhtcy9TQ01Db250YWluZXJQbGFjZW1lbnRSYWNrQXdhcmUuamF2YQ==) | `76.69% <0.00%> (-3.01%)` | `31.00% <0.00%> (-2.00%)` | |
   | [...doop/ozone/container/keyvalue/KeyValueHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIva2V5dmFsdWUvS2V5VmFsdWVIYW5kbGVyLmphdmE=) | `60.88% <0.00%> (-1.12%)` | `62.00% <0.00%> (-1.00%)` | |
   | [...op/ozone/client/io/BlockOutputStreamEntryPool.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL2NsaWVudC9pby9CbG9ja091dHB1dFN0cmVhbUVudHJ5UG9vbC5qYXZh) | `71.79% <0.00%> (-0.65%)` | `34.00% <0.00%> (-1.00%)` | |
   | [.../ozone/container/common/volume/AbstractFuture.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3ZvbHVtZS9BYnN0cmFjdEZ1dHVyZS5qYXZh) | `29.87% <0.00%> (-0.52%)` | `19.00% <0.00%> (-1.00%)` | |
   | [.../org/apache/hadoop/hdds/scm/pipeline/Pipeline.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vcGlwZWxpbmUvUGlwZWxpbmUuamF2YQ==) | `85.23% <0.00%> (-0.48%)` | `44.00% <0.00%> (ø%)` | |
   | [.../apache/hadoop/ozone/om/OmMetadataManagerImpl.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9PbU1ldGFkYXRhTWFuYWdlckltcGwuamF2YQ==) | `82.25% <0.00%> (-0.27%)` | `91.00% <0.00%> (-1.00%)` | |
   | ... and [8 more](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=footer). Last update [57a8388...569cabb](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r446104057



##########
File path: hadoop-hdds/test-utils/src/main/java/org/apache/hadoop/test/JacocoServer.java
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.test;
+
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.net.ServerSocket;
+import java.net.Socket;
+
+import org.jacoco.core.data.ExecutionDataWriter;
+import org.jacoco.core.runtime.RemoteControlReader;
+import org.jacoco.core.runtime.RemoteControlWriter;
+
+/**
+ * Simple TPC server to collect all the Jacoco coverage data.
+ */
+public final class JacocoServer {
+
+  private static int port = 6300;
+
+  private static String destinationFile = "/tmp/jacoco-combined.exec";
+
+  private JacocoServer() {
+  }
+
+  @SuppressWarnings("checkstyle:EmptyStatement")
+  public static void main(String[] args) throws IOException {
+    ExecutionDataWriter destination =
+        new ExecutionDataWriter(new FileOutputStream(destinationFile));
+    ServerSocket serverSocket = new ServerSocket(port);
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+      try {
+        destination.flush();
+        serverSocket.close();
+      } catch (Exception ex) {
+        ex.printStackTrace();
+      }
+    }));
+
+    while (true) {
+      final Socket socket = serverSocket.accept();
+      new Thread(() -> {
+        try {
+          RemoteControlWriter writer =
+              new RemoteControlWriter(socket.getOutputStream());
+          RemoteControlReader reader =
+              new RemoteControlReader(socket.getInputStream());
+          reader.setSessionInfoVisitor(destination::visitSessionInfo);
+          reader.setExecutionDataVisitor(destination::visitClassExecution);
+          while (reader.read()) {
+            ;//read until the end of the stream.
+          }
+          destination.flush();

Review comment:
       Thanks to investigate it. I uploaded a synchronized version. 
   
   The main problem is that the same functionality is written in Jacoco project under LGPL which couldn't be imported. I wrote my own version from scratch, and this mistake clearly proves that it's independent, as I added my own mistakes ;-)




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] codecov-commenter edited a comment on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-643631707


   # [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=h1) Report
   > Merging [#1050](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hadoop-ozone/commit/bd88eec9fec06a8f8adcbe4d257c5ccaca144f4c&el=desc) will **increase** coverage by `0.02%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/graphs/tree.svg?width=650&height=150&src=pr&token=5YeeptJMby)](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1050      +/-   ##
   ============================================
   + Coverage     69.36%   69.38%   +0.02%     
   - Complexity     9097     9103       +6     
   ============================================
     Files           961      961              
     Lines         48127    48127              
     Branches       4676     4676              
   ============================================
   + Hits          33383    33395      +12     
   + Misses        12524    12515       -9     
   + Partials       2220     2217       -3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...hdds/scm/container/common/helpers/ExcludeList.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vY29udGFpbmVyL2NvbW1vbi9oZWxwZXJzL0V4Y2x1ZGVMaXN0LmphdmE=) | `75.51% <0.00%> (-10.21%)` | `18.00% <0.00%> (-3.00%)` | |
   | [...er/common/transport/server/GrpcXceiverService.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3RyYW5zcG9ydC9zZXJ2ZXIvR3JwY1hjZWl2ZXJTZXJ2aWNlLmphdmE=) | `70.00% <0.00%> (-10.00%)` | `3.00% <0.00%> (ø%)` | |
   | [...apache/hadoop/ozone/client/io/KeyOutputStream.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL2NsaWVudC9pby9LZXlPdXRwdXRTdHJlYW0uamF2YQ==) | `78.33% <0.00%> (-2.09%)` | `44.00% <0.00%> (-3.00%)` | |
   | [.../scm/container/AbstractContainerReportHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL2NvbnRhaW5lci9BYnN0cmFjdENvbnRhaW5lclJlcG9ydEhhbmRsZXIuamF2YQ==) | `86.36% <0.00%> (-1.52%)` | `20.00% <0.00%> (-1.00%)` | |
   | [...he/hadoop/ozone/om/ha/OMFailoverProxyProvider.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NvbW1vbi9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL29tL2hhL09NRmFpbG92ZXJQcm94eVByb3ZpZGVyLmphdmE=) | `82.03% <0.00%> (-0.79%)` | `33.00% <0.00%> (-1.00%)` | |
   | [.../transport/server/ratis/ContainerStateMachine.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3RyYW5zcG9ydC9zZXJ2ZXIvcmF0aXMvQ29udGFpbmVyU3RhdGVNYWNoaW5lLmphdmE=) | `69.81% <0.00%> (+0.67%)` | `60.00% <0.00%> (+2.00%)` | |
   | [...doop/ozone/container/keyvalue/KeyValueHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIva2V5dmFsdWUvS2V5VmFsdWVIYW5kbGVyLmphdmE=) | `61.77% <0.00%> (+0.88%)` | `64.00% <0.00%> (+3.00%)` | |
   | [...mon/transport/server/ratis/XceiverServerRatis.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3RyYW5zcG9ydC9zZXJ2ZXIvcmF0aXMvWGNlaXZlclNlcnZlclJhdGlzLmphdmE=) | `92.30% <0.00%> (+1.47%)` | `56.00% <0.00%> (+2.00%)` | |
   | [...ine/commandhandler/DeleteBlocksCommandHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3N0YXRlbWFjaGluZS9jb21tYW5kaGFuZGxlci9EZWxldGVCbG9ja3NDb21tYW5kSGFuZGxlci5qYXZh) | `64.06% <0.00%> (+1.56%)` | `11.00% <0.00%> (+2.00%)` | |
   | [...java/org/apache/hadoop/hdds/utils/db/RDBTable.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvZnJhbWV3b3JrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy91dGlscy9kYi9SREJUYWJsZS5qYXZh) | `61.33% <0.00%> (+2.66%)` | `20.00% <0.00%> (+1.00%)` | |
   | ... and [3 more](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=footer). Last update [bd88eec...b833fef](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-644147318


   Thanks @vivekratnavel I added this flag.
   
   Quick update:
   
   I found that the current approach is not working very well:
   
    * Even if the servers are instrumented by agents I couldn't collect the coverage data as they are persisted only when the JVM is shutting down. With docker, it's hard to do as the `docker-compose down` triggers the JVM shutdown and there is no way to save the jacoco files at that time.
   
   The latest patch tries to follow more generic approach: A tcp server is started (on host) to collect all the coverage data.
   
    1. With this approach we don't need any copy for the jacoco.file-s. It just works
    2. The final jacoco-combined.exec is written by the TCP server.
    3. All the client calls can be measured independent how many client JVM is started
   
   Will see how does it work.


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-649713453


   > If Recon Frontend is not being built, this is possible.
   
   Thanks, I'll give it a try without `-Dskip.npx`.


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r446037856



##########
File path: hadoop-hdds/test-utils/src/main/java/org/apache/hadoop/test/JacocoServer.java
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.test;
+
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.net.ServerSocket;
+import java.net.Socket;
+
+import org.jacoco.core.data.ExecutionDataWriter;
+import org.jacoco.core.runtime.RemoteControlReader;
+import org.jacoco.core.runtime.RemoteControlWriter;
+
+/**
+ * Simple TPC server to collect all the Jacoco coverage data.
+ */
+public final class JacocoServer {
+
+  private static int port = 6300;
+
+  private static String destinationFile = "/tmp/jacoco-combined.exec";
+
+  private JacocoServer() {
+  }
+
+  @SuppressWarnings("checkstyle:EmptyStatement")
+  public static void main(String[] args) throws IOException {
+    ExecutionDataWriter destination =
+        new ExecutionDataWriter(new FileOutputStream(destinationFile));
+    ServerSocket serverSocket = new ServerSocket(port);
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+      try {
+        destination.flush();
+        serverSocket.close();
+      } catch (Exception ex) {
+        ex.printStackTrace();
+      }
+    }));
+
+    while (true) {
+      final Socket socket = serverSocket.accept();
+      new Thread(() -> {
+        try {
+          RemoteControlWriter writer =
+              new RemoteControlWriter(socket.getOutputStream());
+          RemoteControlReader reader =
+              new RemoteControlReader(socket.getInputStream());
+          reader.setSessionInfoVisitor(destination::visitSessionInfo);
+          reader.setExecutionDataVisitor(destination::visitClassExecution);
+          while (reader.read()) {
+            ;//read until the end of the stream.
+          }
+          destination.flush();

Review comment:
       I've found that these calls to `ExecutionDataWriter`'s methods should be [synchronized](https://github.com/adoroszlai/hadoop-ozone/commit/7d21d2c5f7147e879397453f57e3d8d67e91cc99) to avoid producing garbage files, which result in the following exception when trying to read them:
   
   ```
   Exception in thread "main" java.io.UTFDataFormatException: malformed input around byte 2
     at java.io.DataInputStream.readUTF(DataInputStream.java:634)
     at java.io.DataInputStream.readUTF(DataInputStream.java:564)
     at org.jacoco.cli.internal.core.data.ExecutionDataReader.readExecutionData(ExecutionDataReader.java:149)
   ```
   
   or similar `Unknown block type` error.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] vivekratnavel commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-649707779


   @adoroszlai If Recon Frontend is not being built, this is possible. 


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-642638651


   > Let me try to double check the results.
   
   We don't collect all the coverage data right now. This patch collects to exec files after executions of a robot test. It means that only that specific server (eg. scm) will be measured.
   
   I should modify it to collect the files when compose is shutting down...
   
   Another problem (which is harder) to add the measurement from the CLI. But that can be solved later...  


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek merged pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek merged pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050


   


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] vivekratnavel commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-642856876


   @elek Can we skip building Recon UI by adding `-Dskip.yarn` flag to coverage step's build.sh command in both post-commit and pr workflows?
   https://github.com/apache/hadoop-ozone/pull/1050/files#diff-500d30cad2a60d5e5aaa6a95f025ba5fR251
   
   Now that we take arguments for build.sh, this is possible and should save us some time in the CI.


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] codecov-commenter edited a comment on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-643631707


   # [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=h1) Report
   > Merging [#1050](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=desc) into [master](https://codecov.io/gh/apache/hadoop-ozone/commit/57a8388b902a291571bdf60ab3f91930a5cf8815&el=desc) will **increase** coverage by `3.18%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/graphs/tree.svg?width=650&height=150&src=pr&token=5YeeptJMby)](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=tree)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1050      +/-   ##
   ============================================
   + Coverage     70.46%   73.65%   +3.18%     
   - Complexity     9242     9770     +528     
   ============================================
     Files           961      961              
     Lines         48130    48130              
     Branches       4676     4676              
   ============================================
   + Hits          33917    35452    +1535     
   + Misses        11960    10447    -1513     
   + Partials       2253     2231      -22     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
   |---|---|---|---|
   | [...otocol/commands/RetriableDatanodeEventWatcher.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL3Byb3RvY29sL2NvbW1hbmRzL1JldHJpYWJsZURhdGFub2RlRXZlbnRXYXRjaGVyLmphdmE=) | `55.55% <0.00%> (-44.45%)` | `3.00% <0.00%> (-1.00%)` | |
   | [...hdds/scm/container/CloseContainerEventHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvc2VydmVyLXNjbS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL2hkZHMvc2NtL2NvbnRhaW5lci9DbG9zZUNvbnRhaW5lckV2ZW50SGFuZGxlci5qYXZh) | `72.41% <0.00%> (-17.25%)` | `6.00% <0.00%> (ø%)` | |
   | [...e/commandhandler/CreatePipelineCommandHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3N0YXRlbWFjaGluZS9jb21tYW5kaGFuZGxlci9DcmVhdGVQaXBlbGluZUNvbW1hbmRIYW5kbGVyLmphdmE=) | `87.23% <0.00%> (-4.26%)` | `8.00% <0.00%> (ø%)` | |
   | [...apache/hadoop/hdds/server/events/EventWatcher.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvZnJhbWV3b3JrL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zZXJ2ZXIvZXZlbnRzL0V2ZW50V2F0Y2hlci5qYXZh) | `77.77% <0.00%> (-4.17%)` | `14.00% <0.00%> (ø%)` | |
   | [...va/org/apache/hadoop/ozone/lease/LeaseManager.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3Avb3pvbmUvbGVhc2UvTGVhc2VNYW5hZ2VyLmphdmE=) | `90.80% <0.00%> (-2.30%)` | `15.00% <0.00%> (-1.00%)` | |
   | [...ine/commandhandler/DeleteBlocksCommandHandler.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3N0YXRlbWFjaGluZS9jb21tYW5kaGFuZGxlci9EZWxldGVCbG9ja3NDb21tYW5kSGFuZGxlci5qYXZh) | `62.50% <0.00%> (-1.57%)` | `9.00% <0.00%> (-2.00%)` | |
   | [...op/ozone/client/io/BlockOutputStreamEntryPool.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL2NsaWVudC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGFkb29wL296b25lL2NsaWVudC9pby9CbG9ja091dHB1dFN0cmVhbUVudHJ5UG9vbC5qYXZh) | `71.79% <0.00%> (-0.65%)` | `34.00% <0.00%> (-1.00%)` | |
   | [.../org/apache/hadoop/hdds/scm/pipeline/Pipeline.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9oYWRvb3AvaGRkcy9zY20vcGlwZWxpbmUvUGlwZWxpbmUuamF2YQ==) | `85.23% <0.00%> (-0.48%)` | `44.00% <0.00%> (ø%)` | |
   | [.../apache/hadoop/ozone/om/OmMetadataManagerImpl.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLW96b25lL296b25lLW1hbmFnZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9vbS9PbU1ldGFkYXRhTWFuYWdlckltcGwuamF2YQ==) | `82.25% <0.00%> (-0.27%)` | `91.00% <0.00%> (-1.00%)` | |
   | [.../ozone/container/common/volume/AbstractFuture.java](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree#diff-aGFkb29wLWhkZHMvY29udGFpbmVyLXNlcnZpY2Uvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2hhZG9vcC9vem9uZS9jb250YWluZXIvY29tbW9uL3ZvbHVtZS9BYnN0cmFjdEZ1dHVyZS5qYXZh) | `30.64% <0.00%> (+0.25%)` | `21.00% <0.00%> (+1.00%)` | |
   | ... and [218 more](https://codecov.io/gh/apache/hadoop-ozone/pull/1050/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=footer). Last update [57a8388...569cabb](https://codecov.io/gh/apache/hadoop-ozone/pull/1050?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r438640398



##########
File path: hadoop-ozone/dist/src/main/compose/ozone-csi/docker-compose.yaml
##########
@@ -23,6 +23,8 @@ services:
       - ../..:/opt/hadoop
     env_file:
       - docker-config
+    environment:
+      HADOOP_OPTS: ${HADOOP_OPTS}

Review comment:
       We need it for `docker-compose up`. Do you see any possibility to set generic environment variables for all the containers? This approach works only for `docker-compose exec` IMHO.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-642510469


   > I only see 0.7% increase in coverage between master and HDDS-3757 branches. Also, I see several errors related to jacoco.exec in the log:
   
   Yes, I am also surprised. Let me try to double check the results. 


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-649651308


   `HTTP/1.1 403 Forbidden` in Recon Web UI (HDDS-3846) seems to be specific to this change.  Haven't seen it elsewhere.  I'm [running into it](https://github.com/adoroszlai/hadoop-ozone/runs/806226271) even with my changes.


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r446167418



##########
File path: hadoop-hdds/test-utils/src/main/java/org/apache/hadoop/test/JacocoServer.java
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.test;
+
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.net.ServerSocket;
+import java.net.Socket;
+
+import org.jacoco.core.data.ExecutionDataWriter;
+import org.jacoco.core.runtime.RemoteControlReader;
+import org.jacoco.core.runtime.RemoteControlWriter;
+
+/**
+ * Simple TPC server to collect all the Jacoco coverage data.
+ */
+public final class JacocoServer {
+
+  private static int port = 6300;
+
+  private static String destinationFile = "/tmp/jacoco-combined.exec";
+
+  private JacocoServer() {
+  }
+
+  @SuppressWarnings("checkstyle:EmptyStatement")
+  public static void main(String[] args) throws IOException {
+    ExecutionDataWriter destination =
+        new ExecutionDataWriter(new FileOutputStream(destinationFile));
+    ServerSocket serverSocket = new ServerSocket(port);
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+      try {
+        destination.flush();
+        serverSocket.close();
+      } catch (Exception ex) {
+        ex.printStackTrace();
+      }
+    }));
+
+    while (true) {
+      final Socket socket = serverSocket.accept();
+      new Thread(() -> {
+        try {
+          RemoteControlWriter writer =
+              new RemoteControlWriter(socket.getOutputStream());
+          RemoteControlReader reader =
+              new RemoteControlReader(socket.getInputStream());
+          reader.setSessionInfoVisitor(destination::visitSessionInfo);
+          reader.setExecutionDataVisitor(destination::visitClassExecution);
+          while (reader.read()) {
+            ;//read until the end of the stream.
+          }
+          destination.flush();

Review comment:
       `destination.flush();` should be synced, too.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r446037856



##########
File path: hadoop-hdds/test-utils/src/main/java/org/apache/hadoop/test/JacocoServer.java
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ * <p>
+ * http://www.apache.org/licenses/LICENSE-2.0
+ * <p>
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.test;
+
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.net.ServerSocket;
+import java.net.Socket;
+
+import org.jacoco.core.data.ExecutionDataWriter;
+import org.jacoco.core.runtime.RemoteControlReader;
+import org.jacoco.core.runtime.RemoteControlWriter;
+
+/**
+ * Simple TPC server to collect all the Jacoco coverage data.
+ */
+public final class JacocoServer {
+
+  private static int port = 6300;
+
+  private static String destinationFile = "/tmp/jacoco-combined.exec";
+
+  private JacocoServer() {
+  }
+
+  @SuppressWarnings("checkstyle:EmptyStatement")
+  public static void main(String[] args) throws IOException {
+    ExecutionDataWriter destination =
+        new ExecutionDataWriter(new FileOutputStream(destinationFile));
+    ServerSocket serverSocket = new ServerSocket(port);
+    Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+      try {
+        destination.flush();
+        serverSocket.close();
+      } catch (Exception ex) {
+        ex.printStackTrace();
+      }
+    }));
+
+    while (true) {
+      final Socket socket = serverSocket.accept();
+      new Thread(() -> {
+        try {
+          RemoteControlWriter writer =
+              new RemoteControlWriter(socket.getOutputStream());
+          RemoteControlReader reader =
+              new RemoteControlReader(socket.getInputStream());
+          reader.setSessionInfoVisitor(destination::visitSessionInfo);
+          reader.setExecutionDataVisitor(destination::visitClassExecution);
+          while (reader.read()) {
+            ;//read until the end of the stream.
+          }
+          destination.flush();

Review comment:
       I've found that these calls to `ExecutionDataWriter`'s methods should be [synchronized](https://github.com/adoroszlai/hadoop-ozone/commit/7d21d2c5f7147e879397453f57e3d8d67e91cc99) to avoid producing garbage files:
   
   ```
   Exception in thread "main" java.io.UTFDataFormatException: malformed input around byte 2
     at java.io.DataInputStream.readUTF(DataInputStream.java:634)
     at java.io.DataInputStream.readUTF(DataInputStream.java:564)
     at org.jacoco.cli.internal.core.data.ExecutionDataReader.readExecutionData(ExecutionDataReader.java:149)
   ```
   
   or similar `Unknown block type` error.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] vivekratnavel commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
vivekratnavel commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-643666727


   The coverage phase runs a full build and Recon takes ~4 mins to build. This can be optimized by including cache like in build or acceptance stage or by completely excluding Recon UI build by including the flag `-Dskip.npx` flag in build.sh command.


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r438674971



##########
File path: hadoop-ozone/dist/src/main/compose/ozone-csi/docker-compose.yaml
##########
@@ -23,6 +23,8 @@ services:
       - ../..:/opt/hadoop
     env_file:
       - docker-config
+    environment:
+      HADOOP_OPTS: ${HADOOP_OPTS}

Review comment:
       Sorry, I confused the two commands.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-650518259


   Note: if it's stable on the master, we can enable it for the pr builds, too.


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-642860067


   > @elek Can we skip building Recon UI by adding `-Dskip.yarn` flag to coverage step's build.sh command in both post-commit and pr workflows?
   > Now that we take arguments for build.sh, this is possible and should save us some time in the CI.
   
   Seems OK:
   https://github.com/adoroszlai/hadoop-ozone/runs/762799116


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r442235284



##########
File path: hadoop-ozone/dist/src/main/compose/test-all.sh
##########
@@ -19,17 +19,22 @@
 #
 # Test executor to test all the compose/*/test.sh test scripts.
 #
-
 SCRIPT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )
 ALL_RESULT_DIR="$SCRIPT_DIR/result"
-
+PROJECT_DIR="$SCRIPT_DIR/.."
 mkdir -p "$ALL_RESULT_DIR"
-rm "$ALL_RESULT_DIR/*"
+rm "$ALL_RESULT_DIR/*" || true
+
+if [ "$OZONE_WITH_COVERAGE" ]; then
+   java -cp "$PROJECT_DIR"/share/coverage/$(ls "$PROJECT_DIR"/share/coverage | grep test-util):"$PROJECT_DIR"/share/coverage/jacoco-core.jar org.apache.hadoop.test.JacocoServer &
+   DOCKER_BRIDGE_IP=$(docker network inspect bridge --format='{{(index .IPAM.Config 0).Gateway}}')

Review comment:
       Some of the compose environments have a dedicated network.  I think those would have problems starting SCM:
   
   ```
   hadoop-ozone/dist/src/main/compose/ozone-topology/docker-compose.yaml
   hadoop-ozone/dist/src/main/compose/ozonesecure-om-ha/docker-compose.yaml
   hadoop-ozone/dist/src/main/compose/ozonesecure-mr/docker-compose.yaml
   ```
   
   (I'm not sure because I have trouble starting any of the environments with coverage enabled.)

##########
File path: hadoop-ozone/dist/src/main/compose/test-all.sh
##########
@@ -41,8 +46,13 @@ for test in $(find "$SCRIPT_DIR" -name test.sh | sort); do
       echo "ERROR: Test execution of $(dirname "$test") is FAILED!!!!"
   fi
   RESULT_DIR="$(dirname "$test")/result"
-  cp "$RESULT_DIR"/robot-*.xml "$RESULT_DIR"/docker-*.log "$RESULT_DIR"/*.out* "$ALL_RESULT_DIR"/
+  cp "$RESULT_DIR"/robot-*.xml "$RESULT_DIR"/docker-*.log "$RESULT_DIR"/*.out* "$RESULT_DIR"/*.exec "$ALL_RESULT_DIR"/
 done
 
 rebot -N "smoketests" -d "$SCRIPT_DIR/result" "$SCRIPT_DIR/result/robot-*.xml"
+if [ "$OZONE_WITH_COVERAGE" ]; then
+  cp /tmp/jacoco-combined.exec "$SCRIPT_DIR/result/"

Review comment:
       I don't see any `*.exec` files in [acceptance.zip](https://github.com/apache/hadoop-ozone/suites/812019480/artifacts/8877510) for the latest run.  Am I missing something?




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r446102846



##########
File path: hadoop-ozone/dev-support/checks/acceptance.sh
##########
@@ -17,19 +17,32 @@ DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
 cd "$DIR/../../.." || exit 1
 
 REPORT_DIR=${OUTPUT_DIR:-"$DIR/../../../target/acceptance"}
-mkdir -p "$REPORT_DIR"
 
 OZONE_VERSION=$(grep "<ozone.version>" "pom.xml" | sed 's/<[^>]*>//g'|  sed 's/^[ \t]*//')
 DIST_DIR="$DIR/../../dist/target/ozone-$OZONE_VERSION"
 
 if [ ! -d "$DIST_DIR" ]; then
     echo "Distribution dir is missing. Doing a full build"
-    "$DIR/build.sh"
+    "$DIR/build.sh" -Pjacoco
 fi
 
+mkdir -p "$REPORT_DIR"
+
+export HADOOP_OPTS='-javaagent:/opt/hadoop/share/jacoco/jacoco-agent.jar=destfile=/tmp/jacoco.exec,includes=org.apache.hadoop.ozone.*:org.apache.hadoop.hdds'

Review comment:
       I think this is fixed. I prefer to keep the logic in the shell scripts to keep our build as github independent as possible, but with the improved approach this is done (ENV in the github file, logic in the script)




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] adoroszlai commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
adoroszlai commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-649762295


   > If Recon Frontend is not being built, this is possible.
   
   Thanks for the hint.  I can confirm that this was the problem.  The reason I couldn't reproduce it locally was that `node_modules` etc. are not wiped by `mvn clean`.  So despite using `-Dskip.npx`, I did have some Recon UI locally and the test was passing.


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on a change in pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on a change in pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#discussion_r442831540



##########
File path: hadoop-ozone/dist/src/main/compose/test-all.sh
##########
@@ -19,17 +19,22 @@
 #
 # Test executor to test all the compose/*/test.sh test scripts.
 #
-
 SCRIPT_DIR=$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )
 ALL_RESULT_DIR="$SCRIPT_DIR/result"
-
+PROJECT_DIR="$SCRIPT_DIR/.."
 mkdir -p "$ALL_RESULT_DIR"
-rm "$ALL_RESULT_DIR/*"
+rm "$ALL_RESULT_DIR/*" || true
+
+if [ "$OZONE_WITH_COVERAGE" ]; then
+   java -cp "$PROJECT_DIR"/share/coverage/$(ls "$PROJECT_DIR"/share/coverage | grep test-util):"$PROJECT_DIR"/share/coverage/jacoco-core.jar org.apache.hadoop.test.JacocoServer &
+   DOCKER_BRIDGE_IP=$(docker network inspect bridge --format='{{(index .IPAM.Config 0).Gateway}}')

Review comment:
       > (I'm not sure because I have trouble starting any of the environments with coverage enabled.)
   
   How did you try? Docker bridge ip supposed to be available on all the containers. But can be wrong on OSX. But worked well on github environments.




----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org


[GitHub] [hadoop-ozone] elek commented on pull request #1050: HDDS-3757. Add test coverage of the acceptance tests to overall test coverage

Posted by GitBox <gi...@apache.org>.
elek commented on pull request #1050:
URL: https://github.com/apache/hadoop-ozone/pull/1050#issuecomment-650518021


   Thanks the review and the continuous support @adoroszlai (and @vivekratnavel). 
   
   I am merging this now to the master. Weekend seems to be an ideal time for merge this as we can revert in case of any instability (we have 4 scheduled build until Monday)


----------------------------------------------------------------
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.

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



---------------------------------------------------------------------
To unsubscribe, e-mail: ozone-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: ozone-issues-help@hadoop.apache.org