You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@kudu.apache.org by "Edward Fancher (Code Review)" <ge...@cloudera.org> on 2017/12/04 23:42:03 UTC

[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 <ef...@cloudera.com>

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Edward Fancher <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 15 Dec 2017 00:01:31 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Edward Fancher <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 12 Dec 2017 21:53:24 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 12 Dec 2017 23:09:22 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Adar Dembo (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Edward Fancher <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 03 Jan 2018 19:07:51 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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

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 <ef...@cloudera.com>
Gerrit-Reviewer: Edward Fancher <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Tue, 12 Dec 2017 17:36:38 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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

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 <ef...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Edward Fancher <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Mike Percy (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Edward Fancher <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Fri, 12 Jan 2018 02:54:33 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Edward Fancher <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>
Gerrit-Comment-Date: Wed, 13 Dec 2017 23:25:13 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins

[kudu-CR] KUDU-2216. Post process gtest generated xml to include the output from the *.txt files

Posted by "Edward Fancher (Code Review)" <ge...@cloudera.org>.

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 <ef...@cloudera.com>
Gerrit-Reviewer: Edward Fancher <ef...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Todd Lipcon <to...@apache.org>