Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/20355 )
Change subject: IMPALA-12364: Display memory, disk and network metrics in webUI's query timeline ...................................................................... Patch Set 15: (3 comments) http://gerrit.cloudera.org:8080/#/c/20355/13/tests/run-js-tests.sh File tests/run-js-tests.sh: http://gerrit.cloudera.org:8080/#/c/20355/13/tests/run-js-tests.sh@1 PS13, Line 1: #!/bin/sh : : # 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 : # : # http://www.apache.org/licenses/LICENSE-2.0 : # : # 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. For our sanity, always include "set -euo pipefail" at the top of shell scripts. This causes the shell script to fail if any command fails or if there is an unbound variable. We have an add-on that generates a JUnitXML file if this script fails. This can speed up finding the issue when we see a failure in this area. Please add these lines from bin/run-backend-tests.sh: https://github.com/apache/impala/blob/master/bin/run-backend-tests.sh#L21-L22 http://gerrit.cloudera.org:8080/#/c/20355/13/tests/run-js-tests.sh@20 PS13, Line 20: NODEJS_LIB_PATH="${IMPALA_HOME}/lib/node" I don't like our "lib" top level directory, and I intend to remove it at some point. I don't want to add new things there. I think a location in $IMPALA_TOOLCHAIN is a better fit. Also, a good practice is to assume that we will be moving to a new version later, so including the version is useful. i.e. : NODEJS_LIB_PATH="${IMPALA_TOOLCHAIN}/node-${NODEJS_VERSION}" If we do that, then when people are moving back and forth between different branches with different versions, things work appropriately. http://gerrit.cloudera.org:8080/#/c/20355/13/tests/run-js-tests.sh@49 PS13, Line 49: $NPM test We would like output from the tests to end up in ${IMPALA_LOGS_DIR}. For example, we have directories under there for be_tests, ee_tests, custom_cluster_tests, etc. We have a list of all directories here in bin/impala-config.sh: https://github.com/apache/impala/blob/master/bin/impala-config.sh#L844-L863 Is there any way to get this command to produce JUnitXML? We have our Jenkins jobs set up to look for JUnitXML and display the results on the Jenkins job page. This helps a lot when finding failed tests. -- To view, visit http://gerrit.cloudera.org:8080/20355 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifd25e6f0bc9fbd664ec98936daff3f27182dfc7f Gerrit-Change-Number: 20355 Gerrit-PatchSet: 15 Gerrit-Owner: Surya Hebbar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Reviewer: Kurt Deschler <[email protected]> Gerrit-Reviewer: Riza Suminto <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Mon, 25 Sep 2023 22:29:44 +0000 Gerrit-HasComments: Yes
