Wenzhe Zhou has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/20538 )

Change subject: IMPALA-12415: Implement tests for graphical query timeline in 
webUI
......................................................................


Patch Set 4:

(9 comments)

Still got "SyntaxError: Cannot use import statement outside a module" when 
running tests/run-js-tests.sh on my local machine.

http://gerrit.cloudera.org:8080/#/c/20538/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/20538/4//COMMIT_MSG@15
PS4, Line 15: IMPALA_TOOLCHAIN
nit; ${IMPALA_TOOLCHAIN}


http://gerrit.cloudera.org:8080/#/c/20538/4//COMMIT_MSG@25
PS4, Line 25: the profile
where to find profile for these unit-tests?


http://gerrit.cloudera.org:8080/#/c/20538/4//COMMIT_MSG@49
PS4, Line 49: IMPALA_JS_TEST_LOGS_DIR
ni: ${IMPALA_JS_TEST_LOGS_DIR}


http://gerrit.cloudera.org:8080/#/c/20538/4/www/scripts/tests/query_timeline/chart_commons.test.js
File www/scripts/tests/query_timeline/chart_commons.test.js:

http://gerrit.cloudera.org:8080/#/c/20538/4/www/scripts/tests/query_timeline/chart_commons.test.js@24
PS4, Line 24: // Test whether the method correctly searches and maps indexes of 
counters based
            :   // on counter_name
Could we add some negative test cases?


http://gerrit.cloudera.org:8080/#/c/20538/4/www/scripts/tests/query_timeline/fragment_diagram.test.js
File www/scripts/tests/query_timeline/fragment_diagram.test.js:

http://gerrit.cloudera.org:8080/#/c/20538/4/www/scripts/tests/query_timeline/fragment_diagram.test.js@22
PS4, Line 22: are correctly setting attributes
            :   // and returning
nit: correctly set attributes and return


http://gerrit.cloudera.org:8080/#/c/20538/4/www/scripts/tests/query_timeline/fragment_diagram.test.js@26
PS4, Line 26:
nit: add two more spaces


http://gerrit.cloudera.org:8080/#/c/20538/4/www/scripts/tests/query_timeline/fragment_diagram.test.js@47
PS4, Line 47:
nit: indent


http://gerrit.cloudera.org:8080/#/c/20538/4/www/scripts/tests/query_timeline/fragment_diagram.test.js@55
PS4, Line 55:  
nit: indent


http://gerrit.cloudera.org:8080/#/c/20538/4/www/scripts/tests/query_timeline/host_utilization_diagram.test.js
File www/scripts/tests/query_timeline/host_utilization_diagram.test.js:

http://gerrit.cloudera.org:8080/#/c/20538/4/www/scripts/tests/query_timeline/host_utilization_diagram.test.js@17
PS4, Line 17:
Should we add " import {describe, test, expect} from '@jest/globals'; " ?



--
To view, visit http://gerrit.cloudera.org:8080/20538
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I0caf0a0beee23821f78c0b3fe1aeb7dbf92d6a3e
Gerrit-Change-Number: 20538
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Surya Hebbar <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Fri, 20 Oct 2023 23:20:14 +0000
Gerrit-HasComments: Yes

Reply via email to