-
Notifications
You must be signed in to change notification settings - Fork 218
Add style tests #1618
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
Add style tests #1618
Changes from 9 commits
06a5646
dd95739
276f17a
9e92dab
d482ff2
adac638
9056d23
fbac45a
6e62431
613fdcf
286c0e9
4604134
04548c9
d434485
fce593f
215f7ad
34315fa
74a8cf4
41651bf
529af5c
a7991b4
210cc17
5a51dee
e1bf239
854f1dc
5e047ca
d92bb09
a12c72c
795048f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| ## | ||
| # Copyright 2016 Ghent University | ||
| # | ||
| # This file is part of EasyBuild, | ||
| # originally created by the HPC team of Ghent University (http://ugent.be/hpc/en), | ||
| # with support of Ghent University (http://ugent.be/hpc), | ||
| # the Flemish Supercomputer Centre (VSC) (https://vscentrum.be/nl/en), | ||
| # the Hercules foundation (http://www.herculesstichting.be/in_English) | ||
|
||
| # and the Department of Economy, Science and Innovation (EWI) (http://www.ewi-vlaanderen.be/en). | ||
| # | ||
| # http://github.com/hpcugent/easybuild | ||
| # | ||
| # EasyBuild is free software: you can redistribute it and/or modify | ||
| # it under the terms of the GNU General Public License as published by | ||
| # the Free Software Foundation v2. | ||
| # | ||
| # EasyBuild is distributed in the hope that it will be useful, | ||
| # but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| # GNU General Public License for more details. | ||
| # | ||
| # You should have received a copy of the GNU General Public License | ||
| # along with EasyBuild. If not, see <http://www.gnu.org/licenses/>. | ||
| ## | ||
| """ | ||
| Style tests for easyconfig files using pep8. | ||
| @author: Ward Poelmans (Ghent University) | ||
| """ | ||
|
|
||
| import re | ||
| from vsc.utils import fancylogger | ||
|
|
||
| from easybuild.tools.utilities import only_if_module_is_available | ||
|
|
||
| try: | ||
| import pep8 | ||
| except ImportError: | ||
| pass | ||
|
|
||
| _log = fancylogger.getLogger('easyconfig.style', fname=False) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why add a logger if you're not using it? (don't remove it, use it below :)) |
||
|
|
||
|
|
||
| # any function starting with _eb_check_ will be added to the tests | ||
| # if the test number is added to the select list. The test number is | ||
| # definied as AXXX where A is either E or W and XXX is a 3 digit number. | ||
|
||
| # It should be mentioned in the docstring as a single word. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. on a separate line? |
||
| # Read the pep8 docs to understand the arguments of these functions | ||
|
||
| def _eb_check_trailing_whitespace(physical_line, lines, line_number, total_lines): | ||
| """ | ||
| W299 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why shouldn't be define our own range like
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, there is a structure in the codes: https://pep8.readthedocs.org/en/latest/intro.html#error-codes |
||
| Warn about trailing whitespace, expect for the description and comments. | ||
|
||
| This differs from the standard trailing whitespace check as that | ||
| will warn for any trailing whitespace. | ||
| """ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. doc args?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can copy & paste the pep8 docs but I prefer not to. The names are already quite clear and the precise meaning should be looked up in the url above
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please refer to the right place in the pep8 docs in the docstring; the comments won't show up in the auto-generated API documentation for EB we'll have at easybuild.readthedocs.org someday... |
||
| comment_re = re.compile('^\s*#') | ||
| if comment_re.match(physical_line): | ||
| return None | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not a big fan of inline returns (we rarely do this in the current codebase, I'd prefer sticking to that)
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's much more readable in this way. Inline returns are not bad. |
||
|
|
||
| result = pep8.trailing_whitespace(physical_line) | ||
| if result: | ||
| result = (result[0], result[1].replace("W291", "W299")) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ideally, we only have the
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it should be in the docs as well, so it's gonna be here twice.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we string template a docstring? :P
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just tested this, templating on a docstring doesn't really work... |
||
|
|
||
| # if the warning is about the multiline string of description | ||
| # we will not issue a warning | ||
| keys_re = re.compile("^(?P<key>[a-z_]+)\s*=\s*") | ||
|
|
||
| for line in reversed(lines[0:line_number]): | ||
|
||
| res = keys_re.match(line) | ||
| if res: | ||
| if res.group("key") == "description": | ||
| return None | ||
| else: | ||
| break | ||
|
||
|
|
||
| return result | ||
|
|
||
|
|
||
| @only_if_module_is_available('pep8') | ||
| def style_conformance(lst_easyconfigs, verbose=False): | ||
| """ | ||
| Check the given list of easyconfigs for style | ||
| @param lst_easyconfigs list of file paths to easyconfigs | ||
|
||
| @param verbose print our statistics and be verbose about the errors and warning | ||
| @return the number of warnings and errors | ||
| """ | ||
| # importing autopep8 changes some pep8 functions. | ||
| # We reload it to be sure to get the real pep8 functions. | ||
| reload(pep8) | ||
|
|
||
| # register the extra checks before using pep8: | ||
| # any function in this module starting with `_eb_check_` will be used. | ||
| cands = globals() | ||
| for check_function in sorted([cands[f] for f in cands if callable(cands[f]) and f.startswith('_eb_check_')]): | ||
|
||
| pep8.register_check(check_function) | ||
|
|
||
| pep8style = pep8.StyleGuide(quiet=False, config_file=None) | ||
| options = pep8style.options | ||
| options.max_line_length = 120 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a comment above to explain why (and what the default is) |
||
| # we ignore some tests | ||
| # note that W291 has be replaced by our custom W299 | ||
| options.ignore = ('E402', # import not on top | ||
| 'W291', # replaced by W299 | ||
| 'E501', # line too long | ||
|
||
| ) | ||
| options.verbose = int(verbose) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just make the default value
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it is 0? |
||
|
|
||
| result = pep8style.check_files(lst_easyconfigs) | ||
|
|
||
| if verbose: | ||
| result.print_statistics() | ||
|
|
||
| return result.total_errors | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,29 +1,29 @@ | ||
| # should be EB_GCC, but OK for testing purposes | ||
| easyblock = 'EB_toy' | ||
|
|
||
| name="GCC" | ||
| version='4.6.3' | ||
| name = "GCC" | ||
| version = '4.6.3' | ||
|
|
||
| homepage='http://gcc.gnu.org/' | ||
| description="The GNU Compiler Collection includes front ends for C, C++, Objective-C, Fortran, Java, and Ada, as well as libraries for these languages (libstdc++, libgcj,...)." | ||
| homepage = 'http://gcc.gnu.org/' | ||
| description = "The GNU Compiler Collection includes front ends for C, C++, Objective-C, Fortran, Java, and Ada, as well as libraries for these languages (libstdc++, libgcj,...)." | ||
|
|
||
| toolchain={'name': 'dummy','version': 'dummy'} | ||
| toolchain = {'name': 'dummy', 'version': 'dummy'} | ||
|
|
||
| sources=['%s-%s.tar.gz'%(name.lower(),version), | ||
| 'gmp-5.0.4.tar.bz2', | ||
| 'mpfr-3.0.1.tar.gz', | ||
| 'mpc-0.9.tar.gz', | ||
| ] | ||
| source_urls=['http://ftpmirror.gnu.org/%(name)s/%(name)s-%(version)s' % {'name':name.lower(), 'version':version}, # GCC auto-resolving HTTP mirror | ||
| 'http://ftpmirror.gnu.org/gmp', # idem for GMP | ||
| 'http://ftpmirror.gnu.org/mpfr', # idem for MPFR | ||
| 'http://www.multiprecision.org/mpc/download', # MPC official | ||
| sources = ['%s-%s.tar.gz' % (name.lower(), version), | ||
| 'gmp-5.0.4.tar.bz2', | ||
| 'mpfr-3.0.1.tar.gz', | ||
| 'mpc-0.9.tar.gz', | ||
| ] | ||
| source_urls = ['http://ftpmirror.gnu.org/%(name)s/%(name)s-%(version)s' % {'name': name.lower(), 'version': version}, # GCC auto-resolving HTTP mirror | ||
| 'http://ftpmirror.gnu.org/gmp', # idem for GMP | ||
| 'http://ftpmirror.gnu.org/mpfr', # idem for MPFR | ||
| 'http://www.multiprecision.org/mpc/download', # MPC official | ||
| ] | ||
|
||
|
|
||
| languages=['c', 'c++', 'fortran', 'lto'] | ||
| languages = ['c', 'c++', 'fortran', 'lto'] | ||
|
|
||
| ## compiler class | ||
| moduleclass='compiler' | ||
| # compiler class | ||
| moduleclass = 'compiler' | ||
|
|
||
| # building GCC sometimes fails if make parallelism is too high, so let's limit it | ||
| maxparallel=4 | ||
| maxparallel = 4 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,14 +19,14 @@ sources = [SOURCE_TGZ] | |
| # python needs bzip2 to build the bz2 package | ||
| # commented out for testing to avoid having to add them all - dependencies are tested in other files | ||
| dependencies = [ | ||
| # ('bzip2', '1.0.6'), | ||
| # ('zlib', '1.2.8'), | ||
| # ('libreadline', '6.3'), | ||
| # ('ncurses', '5.9'), | ||
| # ('SQLite', '3.8.10.2'), | ||
| # ('Tk', '8.6.4', '-no-X11'), | ||
| # ('OpenSSL', '1.0.1m'), # OS dependency should be preferred if the os version is more recent then this version, it's | ||
| # nice to have an up to date openssl for security reasons | ||
| # ('bzip2', '1.0.6'), | ||
|
||
| # ('zlib', '1.2.8'), | ||
| # ('libreadline', '6.3'), | ||
| # ('ncurses', '5.9'), | ||
| # ('SQLite', '3.8.10.2'), | ||
| # ('Tk', '8.6.4', '-no-X11'), | ||
| # ('OpenSSL', '1.0.1m'), # OS dependency should be preferred if the os version is more recent then this version, it's | ||
| # nice to have an up to date openssl for security reasons | ||
| ] | ||
|
|
||
| # fixme: tuple values; see issue #1442 | ||
|
|
@@ -47,7 +47,7 @@ exts_list = [ | |
| ['numpy', numpyversion, { | ||
| 'source_urls': [['http://sourceforge.net/projects/numpy/files/NumPy/%s' % numpyversion, 'download']], | ||
| 'patches': [ | ||
| 'numpy-1.8.0-mkl.patch', # % numpyversion, | ||
| 'numpy-1.8.0-mkl.patch', # % numpyversion, | ||
| ], | ||
| }], | ||
| ['scipy', scipyversion, { | ||
|
|
@@ -123,4 +123,3 @@ exts_list = [ | |
| ] | ||
|
|
||
| moduleclass = 'lang' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,8 +27,8 @@ sources = ['sqlite-autoconf-%s.tar.gz' % version_str] | |
|
|
||
| # commented out for testing to avoid having to add them all - dependencies are tested in other files | ||
| dependencies = [ | ||
| # ('libreadline', '6.3'), | ||
| # ('Tcl', '8.6.4'), | ||
| # ('libreadline', '6.3'), | ||
| # ('Tcl', '8.6.4'), | ||
|
||
| ] | ||
|
|
||
| parallel = 1 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,8 +27,8 @@ sources = ['sqlite-autoconf-%s.tar.gz' % version_str] | |
|
|
||
| # commented out for testing to avoid having to add them all - dependencies are tested in other files | ||
| dependencies = [ | ||
| # ('libreadline', '6.3'), | ||
| # ('Tcl', '8.6.4'), | ||
| # ('libreadline', '6.3'), | ||
| # ('Tcl', '8.6.4'), | ||
|
||
| ] | ||
|
|
||
| parallel = 1 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,10 +18,10 @@ homepage = "http://www.gzip.org/" | |
| description = "gzip (GNU zip) is a popular data compression program as a replacement for compress" | ||
|
|
||
| # test toolchain specification | ||
| toolchain = {'name': 'GCC','version': '4.6.3'} | ||
| toolchain = {'name': 'GCC', 'version': '4.6.3'} | ||
|
|
||
| # source tarball filename | ||
| sources = ['%s-%s.tar.gz'%(name,version)] | ||
| sources = ['%s-%s.tar.gz' % (name, version)] | ||
|
||
|
|
||
| # download location for source files | ||
| source_urls = [GNU_SOURCE] | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2016-2016 (makes updating trivial next year)