Skip to content

Commit

Permalink
Changes based on github feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
@jgfouca
jgfouca committed Aug 8, 2016
1 parent 1b775e4 commit 9b4488b
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 36 deletions.
50 changes: 24 additions & 26 deletions utils/python/CIME/SystemTests/system_tests_common.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,17 +66,14 @@ def build(self, sharedlib_only=False, model_only=False):

start_time = time.time()
try:
success = self.build_phase(sharedlib_only=(phase_name==SHAREDLIB_BUILD_PHASE),
model_only=(phase_name==MODEL_BUILD_PHASE))
if not isinstance(success, bool):
logger.warning("build_phase did not return a bool, assuming PASS!")
success = True
self.build_phase(sharedlib_only=(phase_name==SHAREDLIB_BUILD_PHASE),
model_only=(phase_name==MODEL_BUILD_PHASE))
except:
success = False

time_taken = time.time() - start_time
with self._test_status:
self._test_status.set_status(phase_name, TEST_PASS_STATUS if success else TEST_FAIL_STATUS, comments=("Time=%d" % int(time_taken)))
self._test_status.set_status(phase_name, TEST_PASS_STATUS if success else TEST_FAIL_STATUS, comments=("time=%d" % int(time_taken)))

if not success:
break
Expand All @@ -89,16 +86,16 @@ def build_phase(self, sharedlib_only=False, model_only=False):
This is the subclass' extension point if they need to define a custom build
phase.
PLEASE THROW EXCEPTION OR RETURN FALSE ON FAIL
PLEASE THROW EXCEPTION ON FAIL
"""
return self.build_indv(sharedlib_only=sharedlib_only, model_only=model_only)
self.build_indv(sharedlib_only=sharedlib_only, model_only=model_only)

def build_indv(self, sharedlib_only=False, model_only=False):
"""
Perform an individual build
"""
return build.case_build(self._caseroot, case=self._case,
sharedlib_only=sharedlib_only, model_only=model_only)
build.case_build(self._caseroot, case=self._case,
sharedlib_only=sharedlib_only, model_only=model_only)

def clean_build(self, comps=None):
build.clean(self._case, cleanlist=comps)
Expand All @@ -116,43 +113,40 @@ def run(self):
with self._test_status:
self._test_status.set_status(RUN_PHASE, TEST_PENDING_STATUS)

success = self.run_phase()
if not isinstance(success, bool):
logger.warning("build_phase did not return a bool, assuming PASS!")
success = True
self.run_phase()

if success:
if self._case.get_value("GENERATE_BASELINE"):
self._generate_baseline()
if self._case.get_value("GENERATE_BASELINE"):
self._generate_baseline()

if self._case.get_value("COMPARE_BASELINE"):
self._compare_baseline()
if self._case.get_value("COMPARE_BASELINE"):
self._compare_baseline()

self._check_for_memleak()
self._check_for_memleak()

except:
success = False
logger.warning("Exception during run: %s" % (sys.exc_info()[1]))

# Always try to report
# Always try to report, should NOT throw an exception
self.report()

# Writing the run status should be the very last thing due to wait_for_tests
time_taken = time.time() - start_time
status = TEST_PASS_STATUS if success else TEST_FAIL_STATUS
with self._test_status:
self._test_status.set_status(RUN_PHASE, status, comments=("TIME=%d" % int(time_taken)))
self._test_status.set_status(RUN_PHASE, status, comments=("time=%d" % int(time_taken)))

return success
# We only return success if every phase, build and later, passed
return self._test_status.get_overall_test_status(ignore_namelists=True) == TEST_PASS_STATUS

def run_phase(self):
"""
This is the default run phase implementation, it just does an individual run.
This is the subclass' extension point if they need to define a custom run phase.
PLEASE THROW AN EXCEPTION OR RETURN FALSE ON FAIL
PLEASE THROW AN EXCEPTION ON FAIL
"""
return self.run_indv()
self.run_indv()

def _set_active_case(self, case):
"""
Expand Down Expand Up @@ -213,10 +207,14 @@ def _component_compare_move(self, suffix):
if rc == 0:
append_status(out, sfile="TestStatus.log")
else:
append_status("Component_compare_test.sh failed out: %s\n\nerr: %s\n" % (out, err),
append_status("Component_compare_move.sh failed out: %s\n\nerr: %s\n" % (out, err),
sfile="TestStatus.log")

def _component_compare_test(self, suffix1, suffix2):
"""
Return value is not generally checked, but is provided in case a custom
run case needs indirection based on success.
"""
cmd = os.path.join(self._case.get_value("SCRIPTSROOT"),"Tools",
"component_compare_test.sh")
rc, out, err = run_cmd("%s -rundir %s -testcase %s -testcase_base %s -suffix1 %s -suffix2 %s -msg 'Compare %s and %s'"
Expand Down
4 changes: 2 additions & 2 deletions utils/python/CIME/wait_for_tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ def get_test_time(test_path):
###############################################################################
ts = TestStatus(test_dir=test_path)
comment = ts.get_comment(RUN_PHASE)
if comment is None or "Time=" not in comment:
if comment is None or "time=" not in comment:
logging.warning("No run-phase time data found in %s" % test_path)
return 0
else:
time_data = [token for token in comment.split() if token.startswith("Time=")][0]
time_data = [token for token in comment.split() if token.startswith("time=")][0]
return int(time_data.split("=")[1])

###############################################################################
Expand Down
2 changes: 1 addition & 1 deletion utils/python/jenkins_generic_job.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def jenkins_generic_job(generate_baselines, submit_to_cdash, no_batch,
else:
cdash_build_name = None

tests_passed = CIME.wait_for_tests.wait_for_tests(glob.glob("%s/*%s*/TestStatus" % (test_root, test_id)),
tests_passed = CIME.wait_for_tests.wait_for_tests(glob.glob("%s/*%s/TestStatus" % (test_root, test_id)),
no_wait=not use_batch, # wait if using queue
check_throughput=False, # don't check throughput
check_memory=False, # don't check memory
Expand Down
21 changes: 14 additions & 7 deletions utils/python/tests/scripts_regression_tests.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

from CIME.utils import run_cmd, run_cmd_no_fail
import update_acme_tests
import CIME.test_scheduler
import CIME.test_scheduler, CIME.wait_for_tests
from CIME.test_scheduler import TestScheduler
from CIME.XML.machines import Machines
from CIME.XML.files import Files
Expand Down Expand Up @@ -549,9 +549,9 @@ def test_b_full(self):
logging.getLogger().setLevel(log_lvl)

if (self._hasbatch):
run_cmd("%s/wait_for_tests *%s*/TestStatus" % (TOOLS_DIR, test_id), from_dir=self._testroot)
run_cmd("%s/wait_for_tests *%s/TestStatus" % (TOOLS_DIR, test_id), from_dir=self._testroot)

test_statuses = glob.glob("%s/*%s*/TestStatus" % (self._testroot, test_id))
test_statuses = glob.glob("%s/*%s/TestStatus" % (self._testroot, test_id))
self.assertEqual(len(tests), len(test_statuses))

for test_status in test_statuses:
Expand Down Expand Up @@ -691,7 +691,7 @@ def simple_test(self, expect_works, extra_args):
if self._hasbatch:
self.assertEqual(stat, 0, msg="COMMAND '%s' SHOULD HAVE WORKED\ncreate_test output:\n%s\n\nerrput:\n%s\n\ncode: %d" % (cmd, output, errput, stat))
test_id = extra_args.split()[extra_args.split().index("-t") + 1]
cmd = "%s/wait_for_tests *%s*/TestStatus" % (TOOLS_DIR, test_id)
cmd = "%s/wait_for_tests *%s/TestStatus" % (TOOLS_DIR, test_id)
stat, output, errput = run_cmd(cmd, from_dir=self._testroot)

if (expect_works):
Expand Down Expand Up @@ -812,7 +812,7 @@ def test_full_system(self):
msg="COMMAND SHOULD HAVE WORKED\ncreate_test output:\n%s\n\nerrput:\n%s\n\ncode: %d" % (output, errput, stat))

if (self._hasbatch):
stat, output, errput = run_cmd("%s/wait_for_tests *%s*/TestStatus" % (TOOLS_DIR, self._baseline_name),
stat, output, errput = run_cmd("%s/wait_for_tests *%s/TestStatus" % (TOOLS_DIR, self._baseline_name),
from_dir=self._testroot)
self.assertEqual(stat, 0,
msg="COMMAND SHOULD HAVE WORKED\nwait_for_tests output:\n%s\n\nerrput:\n%s\n\ncode: %d" % (output, errput, stat))
Expand All @@ -822,6 +822,13 @@ def test_full_system(self):
self.assertEqual(stat, 0,
msg="COMMAND SHOULD HAVE WORKED\ncs.status output:\n%s\n\nerrput:\n%s\n\ncode: %d" % (output, errput, stat))

# Ensure that we can get test times
test_statuses = glob.glob(os.path.join(self._testroot, "*%s" % self._baseline_name, "TestStatus"))
for test_status in test_statuses:
test_time = CIME.wait_for_tests.get_test_time(os.path.dirname(test_status))
self.assertIs(type(test_time), int, msg="get time did not return int for %s" % test_status)
self.assertTrue(test_time > 0, msg="test time was zero for %s" % test_status)

###############################################################################
class TestCimeCase(TestCreateTestCommon):
###############################################################################
Expand Down Expand Up @@ -882,7 +889,7 @@ def test_single_submit(self):
self.assertEqual(stat, 0,
msg="COMMAND SHOULD HAVE WORKED\ncreate_test output:\n%s\n\nerrput:\n%s\n\ncode: %d" % (output, errput, stat))

stat, output, errput = run_cmd("%s/wait_for_tests *%s*/TestStatus -r" % (TOOLS_DIR, self._baseline_name),
stat, output, errput = run_cmd("%s/wait_for_tests *%s/TestStatus -r" % (TOOLS_DIR, self._baseline_name),
from_dir=self._testroot)
self.assertEqual(stat, 0,
msg="COMMAND SHOULD HAVE WORKED\nwait_for_tests output:\n%s\n\nerrput:\n%s\n\ncode: %d" % (output, errput, stat))
Expand All @@ -903,7 +910,7 @@ def test_save_timings(self):
msg="COMMAND SHOULD HAVE WORKED\ncreate_test output:\n%s\n\nerrput:\n%s\n\ncode: %d" % (output, errput, stat))

if (self._hasbatch):
stat, output, errput = run_cmd("%s/wait_for_tests *%s*/TestStatus" % (TOOLS_DIR, self._baseline_name),
stat, output, errput = run_cmd("%s/wait_for_tests *%s/TestStatus" % (TOOLS_DIR, self._baseline_name),
from_dir=self._testroot)
self.assertEqual(stat, 0,
msg="COMMAND SHOULD HAVE WORKED\nwait_for_tests output:\n%s\n\nerrput:\n%s\n\ncode: %d" % (output, errput, stat))
Expand Down

0 comments on commit 9b4488b

Please sign in to comment.