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.

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

DM-15851: Clean target files if we are not performing an install #60

Merged
merged 2 commits into from Oct 15, 2018

Conversation

brianv0
Copy link
Contributor

@brianv0 brianv0 commented Oct 1, 2018

If we are supplying the --last-failed-no-failures flag (lfnfOpt) with the value of all, then make sure to delete the previous target files before executing the SCons command.

If we are invoking SCons for an install, we set the lfnfOpt = "none", otherwise it's all.

Copy link
Member

@timj timj left a comment

Choose a reason for hiding this comment

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

I think the commit messsage (or possibly the code itself) should say that the reason we can't always delete the target files is that if the tests skip for the install we still need the JUnit XML file to be around for Jenkins to collect it. Otherwise, install runs, no tests run, Jenkins thinks no tests ran at all.

In the course of a full build and install, we will typically
invoke SCons twice. The second time we invoke it, it will be for
an install, and "install" will be present in COMMAND_LINE_TARGETS.
If it's not present, we clean up the previous files. If it is,
we keep the previous files for reporting.
@timj
Copy link
Member

timj commented Oct 1, 2018

With this ticket branch I'm now getting junk in my shell when I run scons:

pytest: automated test discovery mode enabled.
@rm -f ${{TARGET}} ${{TARGET}}.failed;
        @printf "%s\n" 'running global pytest... ';
        @({2} TRAVIS=1 {0} {1});         export rc="$?";         if [ "$$rc" -eq 0 ]; then             echo "Global pytest run completed successfully";             cp ${{TARGET}} ${{TARGET}}.all || true;             cp ${{TARGET}}.out ${{TARGET}}.out.all || true;         elif [ "$$rc" -eq 5 ]; then             echo "Global pytest run completed successfully - no tests ran";             mv ${{TARGET}}.all ${{TARGET}} || true;             mv ${{TARGET}}.out.all ${{TARGET}}.out || true;         else             echo "Global pytest run: failed with $$rc";             mv ${{TARGET}}.out ${{TARGET}}.failed;         fi;
        
scons: done reading SConscript files.

Copy link
Member

@timj timj left a comment

Choose a reason for hiding this comment

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

Please fix the spurious output messages and note why this is conditionally needed.

python/lsst/sconsUtils/tests.py Show resolved Hide resolved
@brianv0 brianv0 merged commit b2cbfd6 into master Oct 15, 2018
@brianv0 brianv0 deleted the tickets/DM-15851 branch October 15, 2018 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants