[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8757 ) Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. Patch Set 14: Consider using https://gerrit.cloudera.org/c/8984/ as a template and taking Adar's suggestion to move this logic to run-test.sh -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 14 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 12 Jan 2018 02:54:33 + Gerrit-HasComments: No
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#14). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. This helps when triaging issues in jenkins and other tools that rely on junit output. I included all of the output, since it's difficult to know in advance which parts will be useful. We may want to keep a close eye on Jenkins to see if this results in too much output and we might decide to limit this to just failures at the cost of some increase in parsing complexity. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_gtest_stdout_to_junit_xml.py M build-support/jenkins/build-and-test.sh 2 files changed, 129 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/14 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 14 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Adar Dembo, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#13). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. This helps when triaging issues in jenkins and other tools that rely on junit output. I included all of the output, since it's difficult to know in advance which parts will be useful. We may want to keep a close eye on Jenkins to see if this results in too much output and we might decide to limit this to just failures at the cost of some increase in parsing complexity. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_gtest_stdout_to_junit_xml.py M build-support/jenkins/build-and-test.sh 2 files changed, 129 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/13 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 13 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/8757 ) Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. Patch Set 12: (2 comments) http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py File build-support/jenkins/add_std_out_to_junit.py: http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@68 PS12, Line 68: Globs for *.gz ands *. xml logs in log_location, then sorts the logs I'm missing a ton of context, but why not run this script out of run-test.sh instead of build-and-test.sh? That way it'll run once per test and the precise gz/xml file pair will be known, right? http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@69 PS12, Line 69: I Comment style nit: you'll find the pronouns "we" and "us" more commonly used than "I" and "me". -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 12 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 03 Jan 2018 19:07:51 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Mike Percy has posted comments on this change. (
http://gerrit.cloudera.org:8080/8757 )
Change subject: KUDU-2216. Post process gtest generated xml to include the
output from the *.txt files
..
Patch Set 12:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py
File build-support/jenkins/add_std_out_to_junit.py:
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@1
PS12, Line 1: #!/usr/bin/env python
nit: rename this file to add_gtest_stdout_to_junit_xml.py
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@66
PS12, Line 66: log_location
nit: s/log_location/log_path/ since path is understood to be a string and
location is vague and the object could be something fancy
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@76
PS12, Line 76: "{0}".format(log_location)
seems like a pointless format() call?
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@86
PS12, Line 86: while gz_index < len_gz and xml_index < len_xml:
This lexicographic comparison logic is hard for a reader of the code to follow
in my opinion, even with the code comments. Something along these lines would
reduce the line count and complexity considerably, and be straightforward to
follow:
for xml_file_path in sorted(glob.glob(os.path.join(log_location, "*.xml"))):
# Remove the extension.
file_path_no_ext = os.path.splitext(xml_file_path)[0]
gz_file_path = "%s.gz" % (file_path_no_ext,)
if not os.path.exists(gz_file_path):
continue
tree = ET.parse(xml_filename)
root = tree.getroot()
...
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@90
PS12, Line 90: print ("gz: {0}, xml: {1}".format(gz_filename, xml_filename))
is this for debugging? if so we typically remove these before checking in the
code
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@108
PS12, Line 108: try:
consider using the "with" idiom to auto-close the files
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@109
PS12, Line 109: =
nit: missing space before equals sign
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@135
PS12, Line 135: "{0}".format(xml_files[xml_index])
what is the purpose of using format() here? can't we just pass in xml_file_path
(in the context of my above suggestion)
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@141
PS12, Line 141: # Write out the updated file.
this comment is out of place
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/add_std_out_to_junit.py@148
PS12, Line 148: /
nit: remove the trailing slash because we add a slash later when joining the
path segments
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/build-and-test.sh
File build-support/jenkins/build-and-test.sh:
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/build-and-test.sh@585
PS12, Line 585: t
nit: Finish sentences in code comments with punctuation.
http://gerrit.cloudera.org:8080/#/c/8757/12/build-support/jenkins/build-and-test.sh@586
PS12, Line 586:
nit: extra space
--
To view, visit http://gerrit.cloudera.org:8080/8757
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70
Gerrit-Change-Number: 8757
Gerrit-PatchSet: 12
Gerrit-Owner: Edward Fancher
Gerrit-Reviewer: Edward Fancher
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Fri, 15 Dec 2017 00:01:31 +
Gerrit-HasComments: Yes
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#12). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. This helps when triaging issues in jenkins and other tools that rely on junit output. I included all of the output, since it's difficult to know in advance which parts will be useful. We may want to keep a close eye on Jenkins to see if this results in too much output and we might decide to limit this to just failures at the cost of some increase in parsing complexity. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 154 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/12 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 12 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#11). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. This helps when triaging issues in jenkins and other tools that rely on junit output. I included all of the output, since it's difficult to know in advance which parts will be useful. We may want to keep a close eye on Jenkins to see if this results in too much output and we could decide to limit this to just failures at the cost of some increase in parsing complexity. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 154 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/11 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 11 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Edward Fancher has posted comments on this change. ( http://gerrit.cloudera.org:8080/8757 ) Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. Patch Set 10: (9 comments) http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py File build-support/jenkins/add_std_out_to_junit.py: http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@36 PS7, Line 36: # Matches illegal characters as of the XML 1.0 spec. > In your comment, please explain why we need this. Also, punctuation here an Done http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@39 PS7, Line 39: y unhel > This is a very mechanical name for a variable, almost as unhelpful as namin Done http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@40 PS7, Line 40: # so we need to strip these characters out from the gtest output before > Add docstring here and for process_logs. Done http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@46 PS7, Line 46: Given a suite name and test name, will walk the juni tree to find the > We like to keep our code comments clean, please start with uppercase and en Done http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@49 PS7, Line 49: here. > nit: Mind using "stdout" instead of system-out? I think stdout will be very system-out is what's mentioned in the junit jenkins schema: https://github.com/junit-team/junit5/blob/master/platform-tests/src/test/resources/jenkins-junit.xsd#L70 (That may or may not be authoritative, but it was the best I could find.) http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@63 PS7, Line 63: child.text = ''.join(system_out_node_text_lines) > Mind adding a comment here explaining what this loop is trying to accomplis Done http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@64 PS7, Line 64: > style nit: In comments, capitalize your sentences and end the comment with Done http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@68 PS7, Line 68: obs for *.gz ands *. xml l > what does < mean here? do you mean len(gz_filename) < len(xml_filename) ? I added a comment for that. Basically, since they are sorted lexicographically, I'm using it to keep two running pointers between sorted lists to compare them one at a time, the same way you would for combining two sorted lists. http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@69 PS7, Line 69: lphabetically. I use t > nit: prefer the idiom gz_index += 1 Done -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 10 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 13 Dec 2017 23:25:13 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#10). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. This helps when triaging issues in jenkins and other tools that rely on junit output. I included all of the output, since it's difficult to know in advance which parts will be useful. We may want to keep a close eye on Jenkins to see if this results in too much output and we could decide to limit this to just failures at the cost of some increase in parsing complexity. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 154 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/10 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 10 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#9). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. This helps when triaging issues in jenkins and other tools that rely on junit output. I included all of the output, since it's difficult to know in advance which parts will be useful. We may want to keep a close eye on Jenkins to see if this results in too much output and we could decide to limit this to just failures at the cost of some increase in parsing complexity. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 154 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/9 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 9 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Mike Percy, Jean-Daniel Cryans, Kudu Jenkins, Todd Lipcon, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#8). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. This helps when triaging issues in jenkins and other tools that rely on junit output. I included all of the output, since it's difficult to know in advance which parts will be useful. We may want to keep a close eye on Jenkins to see if this results in too much output and we could decide to limit this to just failures at the cost of some increase in parsing complexity. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 154 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/8 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 8 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8757 ) Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. Patch Set 7: Let's also include tests that crashed in the output -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 7 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 12 Dec 2017 23:09:22 + Gerrit-HasComments: No
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Mike Percy has posted comments on this change. ( http://gerrit.cloudera.org:8080/8757 ) Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. Patch Set 7: (8 comments) http://gerrit.cloudera.org:8080/#/c/8757/7//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/8757/7//COMMIT_MSG@10 PS7, Line 10: The gtest junit output doesn't include the output from the gtest run. Hmm... do we want *all* of the stdout? Isn't that many megabytes in some cases? Are we sure this is what we want? I see that you filed a JIRA to track this. Can you explain why this is useful? http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py File build-support/jenkins/add_std_out_to_junit.py: http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@36 PS7, Line 36: # Matches illegal characters as of the XML 1.1 spec In your comment, please explain why we need this. Also, punctuation here and elsewhere. http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@39 PS7, Line 39: accumul This is a very mechanical name for a variable, almost as unhelpful as naming something "x" or "n". Can you give it a more semantic name? i.e. on a car, instead of naming something "nut", it's better to name it "axle nut". http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@49 PS7, Line 49: system-out nit: Mind using "stdout" instead of system-out? I think stdout will be very clear what it is. http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@63 PS7, Line 63: while gz_index < len_gz and xml_index < len_xml: Mind adding a comment here explaining what this loop is trying to accomplish? Like: # Loop over all of the gz and xml files and extract their contents for inclusion into junit results. or something http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@64 PS7, Line 64: remove the extensions style nit: In comments, capitalize your sentences and end the comment with punctuation such as a period. http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@68 PS7, Line 68: gz_filename < xml_filename what does < mean here? do you mean len(gz_filename) < len(xml_filename) ? http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@69 PS7, Line 69: gz_index = gz_index +1 nit: prefer the idiom gz_index += 1 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 7 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 12 Dec 2017 21:53:24 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Jean-Daniel Cryans has posted comments on this change. ( http://gerrit.cloudera.org:8080/8757 ) Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. Patch Set 7: (2 comments) http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py File build-support/jenkins/add_std_out_to_junit.py: http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@40 PS7, Line 40: node = None Add docstring here and for process_logs. http://gerrit.cloudera.org:8080/#/c/8757/7/build-support/jenkins/add_std_out_to_junit.py@46 PS7, Line 46: # add node with contents of accumul We like to keep our code comments clean, please start with uppercase and end with a period. -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 7 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Jean-Daniel Cryans Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 12 Dec 2017 17:36:38 + Gerrit-HasComments: Yes
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#7). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 119 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/7 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 7 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#6). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 115 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/6 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 6 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#5). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 115 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/5 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 5 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#4). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 116 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/4 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 4 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#3). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 115 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/3 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 3 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Hello Kudu Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/8757 to look at the new patch set (#2). Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 112 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/2 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 2 Gerrit-Owner: Edward Fancher Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files
Edward Fancher has uploaded this change for review. ( http://gerrit.cloudera.org:8080/8757 Change subject: KUDU-2216. Post process gtest generated xml to include the output from the *.txt files .. KUDU-2216. Post process gtest generated xml to include the output from the *.txt files The gtest junit output doesn't include the output from the gtest run. This adds a simple post process parser to pull that information from the gtest result and put it in the junit output. Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 --- A build-support/jenkins/add_std_out_to_junit.py M build-support/jenkins/build-and-test.sh 2 files changed, 112 insertions(+), 0 deletions(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/57/8757/1 -- To view, visit http://gerrit.cloudera.org:8080/8757 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I9f4a9a147f29e37e380cb32ff5af43e1290a1a70 Gerrit-Change-Number: 8757 Gerrit-PatchSet: 1 Gerrit-Owner: Edward Fancher
