Return failure code on failure; DM-1027

[mjuric]

No description provided.

[mjuric]

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

[RobertLuptonTheGood]

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.

[ktlim]

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?

[RobertLuptonTheGood]

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.

[RobertLuptonTheGood]

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

Good to merge?

[mjuric]

Should there be a unit test for this? There probably should as if this code ever gets broken (e.g., does't report failing tests) we should know right away.

[RobertLuptonTheGood]

It was a bit tricky to test this. It involved a complete fake package (with a top-level SConstruct and ./{tests,ups}) living in ./tests.

Pushed.

[mjuric]

+1. Looks good to me. Ship it!

[ktlim]

I don't think you need an external script (as in another file). The Command() in tests.py runs a script that is entirely inline. I'd just like the PRINT_CMD_LINE_FUNC to go away if at all possible. But I'm OK with merging this and fixing that later.

[ktlim]

Oh, in case it wasn't obvious, good to go now.