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.

Sign up for GitHub

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

Jump to bottom

Return failure code on failure; DM-1027 #1

Merged
merged 5 commits into from
Aug 21, 2014
Merged

Return failure code on failure; DM-1027 #1

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
83774cd
Return failure code on failure; DM-1027
RobertLuptonTheGood Aug 15, 2014
57eea23
Fix implementation so that all tests are run, not just the first one.
RobertLuptonTheGood Aug 17, 2014
fcc7be5
Make sure that tests check runs iff the tests are run
RobertLuptonTheGood Aug 17, 2014
1044f8a
Added a test that we check the error code of tests
RobertLuptonTheGood Aug 19, 2014
ceefb6f
Use shell, not python, for testing so we can use @
RobertLuptonTheGood Aug 21, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions python/lsst/sconsUtils/scripts.py
View file
Open in desktop
Expand Up @@ -148,6 +148,29 @@ def finish(defaultTargets=("lib", "python", "tests", "examples", "doc"),
state.env.Default(state.targets["version"])
state.env.Requires(state.targets["tests"], state.targets["version"])
state.env.Decider("MD5-timestamp") # if timestamps haven't changed, don't do MD5 checks
#
# Just before scons exits check if any of the tests failed.
#
if "tests" in [str(t) for t in BUILD_TARGETS]:
def __checkTestStatus( target, source, env ):
"""See if any tests failed"""
import glob
nFailed = len(glob.glob(os.path.join(os.getcwd(), "tests", ".tests", "*.failed")))
if nFailed > 0:
raise SCons.Errors.BuildError(errstr="%d tests failed" % nFailed)

checkTestStatus_command = state.env.Command('checkTestStatus', [], __checkTestStatus)

state.env.Depends(checkTestStatus_command, BUILD_TARGETS) # this is why the check runs last
BUILD_TARGETS.extend(checkTestStatus_command)
state.env.AlwaysBuild(checkTestStatus_command)
#
# We don't want to clutter up the output with our check for failed tests
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels a bit hacky. Is this a standard scons idiom?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit doesn't bother me (setting PRINT_CMD_LINE_FUNC does --- if it were a unix command I could just prefix it with @ like in make).

I create an Action (checkTestStatus_command) and ask that it always run, but only when everything else has finished. I could register an atexit handler to do this (at the python level) but by then it's too late to just call sys.exit(1) --- the multiprocess cleanup complains.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't you have the Command action be an actual shell script to look for and count the ".failed" files, thus enabling it to be "@"-prefixed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought that an external script was an even worse solution. I should ask the scons list if there's a better solution (a keyword arg?)

    R

On 19 Aug 2014, at 04:30, ktlim notifications@github.com wrote:

In python/lsst/sconsUtils/scripts.py:

  •    #

  •    if "tests" in [str(t) for t in BUILD_TARGETS]:

  •        def __checkTestStatus( target, source, env ):

  •            """See if any tests failed"""

  •            import glob

  •            nFailed = len(glob.glob(os.path.join(os.getcwd(), "tests", ".tests", "*.failed")))

  •            if nFailed > 0:

  •                raise SCons.Errors.BuildError(errstr="%d tests failed" % nFailed)

  •        checkTestStatus_command = state.env.Command('checkTestStatus', [], __checkTestStatus)

  •        state.env.Depends(checkTestStatus_command, BUILD_TARGETS) # this is why the check runs last

  •        BUILD_TARGETS.extend(checkTestStatus_command)

  •        state.env.AlwaysBuild(checkTestStatus_command)

  •        #

  •        # We don't want to clutter up the output with our check for failed tests
    Can't you have the Command action be an actual shell script to look for and count the ".failed" files, thus enabling it to be "@"-prefixed?

—
Reply to this email directly or view it on GitHub.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, replaced python test with inline shell as per KT's request and stopped messing with PRINT_CMD_LINE_FUNC

Good to merge?

#
def print_cmd_line(s, target, source, env):
if not s.startswith("__checkTestStatus"):
sys.stdout.write(s + "\n")
state.env['PRINT_CMD_LINE_FUNC'] = print_cmd_line

##
# @brief A scope-only class for SConscript-replacement convenience functions.
Expand Down
2 changes: 1 addition & 1 deletion python/lsst/sconsUtils/tests.py
View file
Open in desktop
Expand Up @@ -173,5 +173,5 @@ def run(self, fileGlob):
self._env.Alias(os.path.basename(target), target)

self._env.Clean(target, self._tmpDir)

return targets