Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TravisCI: Count Errors and Warnings. #296

Merged
merged 7 commits into from Apr 26, 2016
Merged

Conversation

@tammoippen
Copy link
Contributor

@tammoippen tammoippen commented Apr 7, 2016

This PR extends the parsing of TravisCI compiler output for number of warnings and errors.

if line.strip() == '+make install':
return warn, error, line

# /home/travis/build/tammoippen/nest-simulator/build/libnestutil/config.h:65:0: warning: "HAVE_DIRENT_H" redefined [enabled by default]

This comment has been minimized.

@heplesser

heplesser Apr 11, 2016
Contributor

Should this comment be removed?

This comment has been minimized.

@tammoippen

tammoippen Apr 13, 2016
Author Contributor

Also in the other parse functions, I have the line I want match included as a comment. For consistency, I should remove them there as well.

This comment has been minimized.

@jougs

jougs Apr 25, 2016
Contributor

Please remove them (or at least make it explicit that they are the expected output). Thanks!

warn[file] = 0
warn[file] += 1

if ': error:' in line:

This comment has been minimized.

@heplesser

heplesser Apr 11, 2016
Contributor

I am not really fond of a string constant here, as compilers may change. But gcc 4 and 5, clang, Intel and Portland Group compilers all include this string in their error messages, so I guess it is a rather safe bet. xlC will not work, but I think that is not a problem for checking on Travis.

This comment has been minimized.

@tammoippen

tammoippen Apr 13, 2016
Author Contributor

I know what you mean, but as the purpose of this script is to parse TravisCI output, i think it is ok to assume gcc 4 and 5.

@heplesser
Copy link
Contributor

@heplesser heplesser commented Apr 11, 2016

@tammoippen Looks mostly fine, but I have two issues.

First, CMake hides warnings from us, except those that come from the pynest build process. I think we need to use make VERBOSE=1 to see the warnings.

Second, towards the end of the build log, I see

Built target installcheck
+[  !=  ]
+[ 296 != false ]
+echo WARNING: Not uploading results as this is a pull request
WARNING: Not uploading results as this is a pull request
+exit 0

What are the two lines with the square brackets and inequality doing there?

@tammoippen
Copy link
Contributor Author

@tammoippen tammoippen commented Apr 11, 2016

@heplesser The two lines are generated by build.sh.

@tammoippen
Copy link
Contributor Author

@tammoippen tammoippen commented Apr 13, 2016

I put this PR on a hold, as the warnings are not parsed... I have to investigate.

Edit: Found the problem. ac5a7ed should parse the output again.

@tammoippen tammoippen force-pushed the tammoippen:count_warnings branch from ab879bf to ac5a7ed Apr 13, 2016
@tammoippen
Copy link
Contributor Author

@tammoippen tammoippen commented Apr 18, 2016

@heplesser Can you please have another look?

@heplesser
Copy link
Contributor

@heplesser heplesser commented Apr 18, 2016

👍

@tammoippen
Copy link
Contributor Author

@tammoippen tammoippen commented Apr 20, 2016

@jougs Can you please look into this PR? Thank you.


# /home/travis/build/tammoippen/nest-simulator/build/libnestutil/config.h:65:0: warning: "HAVE_DIRENT_H" redefined [enabled by default]
if ': warning:' in line:
file = line.split(':')[0]

This comment has been minimized.

@jakobj

jakobj Apr 25, 2016
Contributor

i'm fairly unhappy about the choice of using file as a variable name here, as this is also a Python object. I would use file_name instead.

@tammoippen tammoippen force-pushed the tammoippen:count_warnings branch from 16a5d64 to cfbea83 Apr 25, 2016
@tammoippen tammoippen force-pushed the tammoippen:count_warnings branch from cfbea83 to 3c9f5a5 Apr 25, 2016
print("Make install: " + ("Ok" if make_install_ok else "Error"))
print("Make installcheck: " + ("Ok (" if make_installcheck_failed == 0 else "Error (") +
str(make_installcheck_failed) + " / " + str(make_installcheck_all) + ")")
print("Logs uploaded to S3: " + ("Yes" if uploading_results else "No"))

print("\nStatic analysis:" )
print_static_analysis(static_analysis)

print("\n\nWarnings:")
for k,v in actual_warnings.iteritems():

This comment has been minimized.

@jougs

jougs Apr 25, 2016
Contributor

There's a space missing between "k," and "v" for PEP-8 compliance.

@jougs
Copy link
Contributor

@jougs jougs commented Apr 25, 2016

Nice work. 👍 after my comment is addressed

@jougs jougs merged commit 07caea3 into nest:master Apr 26, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tammoippen tammoippen deleted the tammoippen:count_warnings branch May 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.