Skip to content

Commit 367d47f

Browse files
committed
Merge branch 'jgfouca/add_code_checking' (PR #267)
Add a new tool that checks that all python library source files are 100% clean for a certain configuration of pylint. Fix all pylint errors discovered by the tool. Add a test to run this tool every night. Test suite: scripts_regression_tests Test baseline: Test namelist changes: Test status: bit for bit Fixes: [CIME Github issue #] User interface changes?: Code review: @jedwards4b * jgfouca/add_code_checking: Fix a couple remaining pylint issues Made code_checker parallel, fix test name so it actually runs Fix remaining test failures Bug fix All scripts passing Add code checker test. Remove refactor disablings from scripts_regression_tests.py Progress. Tool added. build.py 100% Conflicts: utils/python/CIME/case_setup.py
2 parents e46d0f4 + de33082 commit 367d47f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

71 files changed

+584
-510
lines changed

driver_cpl/cime_config/buildexe

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,19 +42,15 @@ def _main_func():
4242
# build model executable
4343

4444
makefile = os.path.join(casetools, "Makefile")
45-
exename = os.path.join(exeroot, model + ".exe")
45+
exename = os.path.join(exeroot, model + ".exe")
4646

4747
cmd = "%s exec_se -j %d EXEC_SE=%s MODEL=%s LIBROOT=%s -f %s "\
4848
% (gmake, gmake_j, exename, "driver", libroot, makefile)
4949

50-
rc, out, err = run_cmd(cmd, ok_to_fail=True)
50+
rc, out, err = run_cmd(cmd)
5151
expect(rc==0,"Command %s failed rc=%d\nout=%s\nerr=%s"%(cmd,rc,out,err))
5252

5353
###############################################################################
5454

5555
if __name__ == "__main__":
5656
_main_func()
57-
58-
59-
60-

driver_cpl/cime_config/buildnml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ def _main_func():
6868
cmd = cmd + " -grid %s -atm_grid %s -lnd_grid %s -rof_grid %s -ocn_grid %s -wav_grid %s" \
6969
% (grid, atm_grid, lnd_grid, rof_grid, ocn_grid, wav_grid)
7070

71-
rc, out, err = run_cmd(cmd, from_dir=confdir, ok_to_fail=True)
71+
rc, out, err = run_cmd(cmd, from_dir=confdir)
7272
expect(rc==0,"Command %s failed rc=%d\nout=%s\nerr=%s"%(cmd,rc,out,err))
7373

7474
# copy drv_in, seq_maps.rc and all *modio* files to rundir

scripts/Tools/acme_bisect

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ script is intended to be run by git bisect.
66
"""
77

88
from standard_script_setup import *
9-
from CIME.utils import expect, run_cmd
9+
from CIME.utils import expect, run_cmd_no_fail
1010
from CIME.XML.machines import Machines
1111

1212
import argparse, sys, os, doctest, traceback
@@ -80,18 +80,18 @@ def acme_bisect(testname, good, bad, testroot, compiler, project, baseline_name,
8080
wait_for_tests = os.path.join(CIME.utils.get_scripts_root(), "Tools", "wait_for_tests")
8181

8282
# Important: we only want to test merges
83-
commits_we_want_to_test = run_cmd("git rev-list %s..%s --merges --first-parent" % (good, bad)).splitlines()
84-
all_commits = run_cmd("git rev-list %s..%s" % (good, bad)).splitlines()
83+
commits_we_want_to_test = run_cmd_no_fail("git rev-list %s..%s --merges --first-parent" % (good, bad)).splitlines()
84+
all_commits = run_cmd_no_fail("git rev-list %s..%s" % (good, bad)).splitlines()
8585
commits_to_skip = set(all_commits) - set(commits_we_want_to_test)
8686
print "Skipping these non-merge commits"
8787
for item in commits_to_skip:
8888
print item
8989

9090
# Basic setup
91-
run_cmd("git bisect start")
92-
run_cmd("git bisect good %s" % good)
93-
run_cmd("git bisect bad %s" % bad)
94-
run_cmd("git bisect skip %s" % " ".join(commits_to_skip))
91+
run_cmd_no_fail("git bisect start")
92+
run_cmd_no_fail("git bisect good %s" % good)
93+
run_cmd_no_fail("git bisect bad %s" % bad)
94+
run_cmd_no_fail("git bisect skip %s" % " ".join(commits_to_skip))
9595

9696
# Formulate the create_test command
9797

@@ -114,9 +114,9 @@ def acme_bisect(testname, good, bad, testroot, compiler, project, baseline_name,
114114

115115
bisect_cmd += " && %s" % wait_for_tests_cmd
116116

117-
run_cmd("git bisect run sh -c '%s'" % bisect_cmd, ok_to_fail=True, verbose=True)
117+
run_cmd("git bisect run sh -c '%s'" % bisect_cmd, verbose=True)
118118

119-
run_cmd("git bisect reset")
119+
run_cmd_no_fail("git bisect reset")
120120

121121
###############################################################################
122122
def _main_func(description):

scripts/Tools/bless_test_results

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ from standard_script_setup import *
1414

1515
import wait_for_tests, compare_namelists, simple_compare
1616
from CIME.system_test import NAMELIST_PHASE
17-
from CIME.utils import run_cmd, expect
17+
from CIME.utils import run_cmd, run_cmd_no_fail, expect
1818
from CIME.XML.machines import Machines
1919

2020
import argparse, sys, os, glob, doctest, time
@@ -135,7 +135,7 @@ def bless_namelists(test_name, baseline_dir_for_test, testcase_dir_for_test, rep
135135
def bless_history(test_name, baseline_tag, baseline_dir_for_test, testcase_dir_for_test, report_only, force):
136136
###############################################################################
137137
# Get user that test was run as (affects loc of hist files)
138-
acme_root = run_cmd("./xmlquery CESMSCRATCHROOT -value", from_dir=testcase_dir_for_test)
138+
acme_root = run_cmd_no_fail("./xmlquery CESMSCRATCHROOT -value", from_dir=testcase_dir_for_test)
139139

140140
case = os.path.basename(testcase_dir_for_test)
141141
cime_root = CIME.utils.get_cime_root()
@@ -147,15 +147,15 @@ def bless_history(test_name, baseline_tag, baseline_dir_for_test, testcase_dir_f
147147
(machine_env, compgen, baseline_dir_for_test, case, test_name, run_dir, cprnc_loc, baseline_tag)
148148

149149
check_compare = os.path.join(cime_root, "scripts", "Tools", "component_write_comparefail.pl")
150-
stat, out, _ = run_cmd("%s %s 2>&1" % (check_compare, run_dir), ok_to_fail=True)
150+
stat, out, _ = run_cmd("%s %s 2>&1" % (check_compare, run_dir))
151151

152152
if (stat != 0):
153153
# found diff, offer rebless
154154
print out
155155

156156
if (not report_only and
157157
(force or raw_input("Update this diff (y/n)? ").upper() in ["Y", "YES"])):
158-
stat = run_cmd(compgen_cmd, ok_to_fail=True, verbose=True)[0]
158+
stat = run_cmd(compgen_cmd, verbose=True)[0]
159159
if (stat != 0):
160160
logging.warning("Hist file bless FAILED for test %s" % test_name)
161161
return False, "Bless command failed"

scripts/Tools/case.submit

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,11 @@ OR
4444

4545
args = parser.parse_args(args[1:])
4646

47+
CIME.utils.expect(args.prereq is None, "--prereq not currently supported")
48+
4749
CIME.utils.handle_standard_logging_options(args)
4850

49-
return args.caseroot, args.job, args.no_batch, args.prereq
51+
return args.caseroot, args.job, args.no_batch
5052

5153
###############################################################################
5254
def _main_func(description):
@@ -55,9 +57,9 @@ def _main_func(description):
5557
test_results = doctest.testmod(verbose=True)
5658
sys.exit(1 if test_results.failed > 0 else 0)
5759

58-
caseroot, job, no_batch, prereq = parse_command_line(sys.argv, description)
60+
caseroot, job, no_batch = parse_command_line(sys.argv, description)
5961
with Case(caseroot, read_only=False) as case:
60-
submit(case, job=job, no_batch=no_batch, prereq_jobid=prereq)
62+
submit(case, job=job, no_batch=no_batch)
6163

6264
if __name__ == "__main__":
6365
_main_func(__doc__)

scripts/Tools/check_input_data

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ Should be run from case.
77
"""
88
from standard_script_setup import *
99

10-
from CIME.utils import expect, get_model, run_cmd
10+
from CIME.utils import expect, get_model
1111
from CIME.XML.machines import Machines
1212
from CIME.check_input_data import check_input_data, SVN_LOCS
1313
from CIME.case import Case

scripts/Tools/code_checker

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
#!/usr/bin/env python
2+
3+
"""
4+
Ensure that all CIME python files are free of errors
5+
and follow the PEP8 standard.
6+
"""
7+
8+
from standard_script_setup import *
9+
10+
from CIME.utils import run_cmd, run_cmd_no_fail, expect, get_python_libs_root
11+
12+
import argparse, sys, os, doctest
13+
from multiprocessing.dummy import Pool as ThreadPool
14+
15+
###############################################################################
16+
def parse_command_line(args, description):
17+
###############################################################################
18+
parser = argparse.ArgumentParser(
19+
usage="""\n%s [--verbose]
20+
OR
21+
%s --help
22+
OR
23+
%s --test
24+
25+
\033[1mEXAMPLES:\033[0m
26+
\033[1;32m# Check code \033[0m
27+
> %s
28+
""" % ((os.path.basename(args[0]), ) * 4),
29+
30+
description=description,
31+
32+
formatter_class=argparse.ArgumentDefaultsHelpFormatter
33+
)
34+
35+
CIME.utils.setup_standard_logging_options(parser)
36+
37+
parser.add_argument("--dir", default=get_python_libs_root(),
38+
help="The root directory containing python files to check.")
39+
40+
parser.add_argument("-j", "--num-procs", type=int, default=8,
41+
help="The number of files to check in parallel")
42+
43+
args = parser.parse_args(args[1:])
44+
45+
CIME.utils.handle_standard_logging_options(args)
46+
47+
return args.dir, args.num_procs
48+
49+
###############################################################################
50+
def run_pylint(on_file):
51+
###############################################################################
52+
cmd = "pylint --disable I,C,R,logging-not-lazy,wildcard-import,unused-wildcard-import,fixme,broad-except,bare-except,eval-used,exec-used,global-statement %s" % on_file
53+
stat = run_cmd(cmd)[0]
54+
if stat != 0:
55+
sys.stdout.write("File %s has pylint problems, please fix\n Use command: %s\n" % (on_file, cmd))
56+
return False
57+
else:
58+
sys.stdout.write("File %s has no pylint problems\n" % on_file)
59+
return True
60+
61+
###############################################################################
62+
def check_code(dir_to_check, num_procs):
63+
###############################################################################
64+
"""
65+
Check all python files in the given directory
66+
67+
Returns True if all files had no problems
68+
"""
69+
# Pylint won't work right if the imports within the checked files fails
70+
if "PYTHONPATH" in os.environ:
71+
os.environ["PYTHONPATH"] += dir_to_check
72+
else:
73+
os.environ["PYTHONPATH"] = dir_to_check
74+
75+
# Get list of files to check
76+
files_to_check = run_cmd_no_fail('find %s -name "*.py"' % dir_to_check).splitlines()
77+
pool = ThreadPool(num_procs)
78+
results = pool.map(run_pylint, files_to_check)
79+
80+
return False not in results
81+
82+
###############################################################################
83+
def _main_func(description):
84+
###############################################################################
85+
if ("--test" in sys.argv):
86+
test_results = doctest.testmod(verbose=True)
87+
sys.exit(1 if test_results.failed > 0 else 0)
88+
89+
dir_to_check, num_procs = parse_command_line(sys.argv, description)
90+
91+
sys.exit(0 if check_code(dir_to_check, num_procs) else 1)
92+
93+
###############################################################################
94+
95+
if (__name__ == "__main__"):
96+
_main_func(__doc__)

scripts/Tools/compare_namelists

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ formatter_class=argparse.ArgumentDefaultsHelpFormatter
5454
def _main_func(description):
5555
###############################################################################
5656
if ("--test" in sys.argv):
57-
CIME.utils.run_cmd("python -m doctest %s/compare_namelists.py -v" % CIME.utils.get_python_libs_root(), arg_stdout=None, arg_stderr=None)
57+
CIME.utils.run_cmd_no_fail("python -m doctest %s/compare_namelists.py -v" % CIME.utils.get_python_libs_root(), arg_stdout=None, arg_stderr=None)
5858
return
5959

6060
gold_file, compare_file, case = \

0 commit comments

Comments
 (0)