* travis: any reason we have keep going on make commands @ 2020-07-09 14:33 William Roberts 2020-07-09 18:00 ` Paul Moore 0 siblings, 1 reply; 7+ messages in thread From: William Roberts @ 2020-07-09 14:33 UTC (permalink / raw) To: SElinux list, Nicolas Iooss, Stephen Smalley So Nicolas initially created our travis script in commit c9adfe2d2653 and has -k, or keep going, on the make commands. This causes make to plow ahead and bury the errors in the logs. Stephen noticed this the other day, and we have been chatting about it out of band and wanted to pull in the community. Are their compelling reasons for keeping this behavior? I am also concerned that we could get false positives on travis success results. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: travis: any reason we have keep going on make commands 2020-07-09 14:33 travis: any reason we have keep going on make commands William Roberts @ 2020-07-09 18:00 ` Paul Moore 2020-07-10 9:08 ` Nicolas Iooss 0 siblings, 1 reply; 7+ messages in thread From: Paul Moore @ 2020-07-09 18:00 UTC (permalink / raw) To: William Roberts; +Cc: SElinux list, Nicolas Iooss, Stephen Smalley On Thu, Jul 9, 2020 at 10:33 AM William Roberts <bill.c.roberts@gmail.com> wrote: > So Nicolas initially created our travis script in commit c9adfe2d2653 > and has -k, or keep going, on the make commands. This causes make to > plow ahead and bury the errors in the logs. Stephen noticed this the > other day, and we have been chatting about it out of band and wanted > to pull in the community. > > Are their compelling reasons for keeping this behavior? I am also > concerned that we could get false positives on travis success results. In my opinion the whole point of automated testing is to catch failures early and often. For that reason I would want the test to fail and stop, both because I find it easier to identify the failure that way and also because I'm not sure I would trust much of the testing that occurred after an error condition. -- paul moore www.paul-moore.com ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: travis: any reason we have keep going on make commands 2020-07-09 18:00 ` Paul Moore @ 2020-07-10 9:08 ` Nicolas Iooss 2020-07-10 15:22 ` William Roberts 0 siblings, 1 reply; 7+ messages in thread From: Nicolas Iooss @ 2020-07-10 9:08 UTC (permalink / raw) To: Paul Moore, William Roberts; +Cc: SElinux list, Stephen Smalley On Thu, Jul 9, 2020 at 8:00 PM Paul Moore <paul@paul-moore.com> wrote: > > On Thu, Jul 9, 2020 at 10:33 AM William Roberts > <bill.c.roberts@gmail.com> wrote: > > So Nicolas initially created our travis script in commit c9adfe2d2653 > > and has -k, or keep going, on the make commands. This causes make to > > plow ahead and bury the errors in the logs. Stephen noticed this the > > other day, and we have been chatting about it out of band and wanted > > to pull in the community. > > > > Are their compelling reasons for keeping this behavior? I am also > > concerned that we could get false positives on travis success results. > > In my opinion the whole point of automated testing is to catch > failures early and often. For that reason I would want the test to > fail and stop, both because I find it easier to identify the failure > that way and also because I'm not sure I would trust much of the > testing that occurred after an error condition. > Hi, There seems to be some confusion: * "make -k" does not stop the "make" command at the first error and allows seeing all the errors when there are several ones. In my humble opinion, it makes sense when compiling ("make all") and not when running tests ("make test"), and this is actually what is right now in Travis-CI. "make -k" returns a failure exit code when an error happens. * Travis-CI does not stop the job as soon as a sub-command fails. If I understand correctly, this is what really bothers William, and I agree this is a behavior that can be improved. According to https://github.com/travis-ci/travis-ci/issues/1066, a possible solution could be to use "set -e", which could have unexpected side-effects in launched commands. It is possible to "emulate set -e" by adding exit statements, such as : - make install $EXPLICIT_MAKE_VARS -k || exit $? - make install-pywrap $EXPLICIT_MAKE_VARS -k || exit $? - make install-rubywrap $EXPLICIT_MAKE_VARS -k || exit $? # ... - make test $EXPLICIT_MAKE_VARS || exit $? I have not tested whether this works on Travis-CI, but if it does, it would be a nice improvement. I will take a look this week-end. Cheers, Nicolas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: travis: any reason we have keep going on make commands 2020-07-10 9:08 ` Nicolas Iooss @ 2020-07-10 15:22 ` William Roberts 2020-07-11 16:56 ` Nicolas Iooss 0 siblings, 1 reply; 7+ messages in thread From: William Roberts @ 2020-07-10 15:22 UTC (permalink / raw) To: Nicolas Iooss; +Cc: Paul Moore, SElinux list, Stephen Smalley On Fri, Jul 10, 2020 at 4:10 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > On Thu, Jul 9, 2020 at 8:00 PM Paul Moore <paul@paul-moore.com> wrote: > > > > On Thu, Jul 9, 2020 at 10:33 AM William Roberts > > <bill.c.roberts@gmail.com> wrote: > > > So Nicolas initially created our travis script in commit c9adfe2d2653 > > > and has -k, or keep going, on the make commands. This causes make to > > > plow ahead and bury the errors in the logs. Stephen noticed this the > > > other day, and we have been chatting about it out of band and wanted > > > to pull in the community. > > > > > > Are their compelling reasons for keeping this behavior? I am also > > > concerned that we could get false positives on travis success results. > > > > In my opinion the whole point of automated testing is to catch > > failures early and often. For that reason I would want the test to > > fail and stop, both because I find it easier to identify the failure > > that way and also because I'm not sure I would trust much of the > > testing that occurred after an error condition. > > > Hi, > There seems to be some confusion: > > * "make -k" does not stop the "make" command at the first error and > allows seeing all the errors when there are several ones. In my humble > opinion, it makes sense when compiling ("make all") and not when > running tests ("make test"), and this is actually what is right now in > Travis-CI. "make -k" returns a failure exit code when an error > happens. Ahh I thought it returned to 0. Not sure why I assumed that. > > * Travis-CI does not stop the job as soon as a sub-command fails. If I Depends on the stage: https://docs.travis-ci.com/user/job-lifecycle/ If before_install, install or before_script returns a non-zero exit code, the build is errored and stops immediately. If script returns a non-zero exit code, the build is failed, but continues to run before being marked as failed. I put a false command in the script section and it kept plowing ahead as you foretold. > understand correctly, this is what really bothers William, and I agree > this is a behavior that can be improved. According to > https://github.com/travis-ci/travis-ci/issues/1066, a possible > solution could be to use "set -e", which could have unexpected > side-effects in launched commands. It is possible to "emulate set -e" > by adding exit statements, such as : > > - make install $EXPLICIT_MAKE_VARS -k || exit $? > - make install-pywrap $EXPLICIT_MAKE_VARS -k || exit $? > - make install-rubywrap $EXPLICIT_MAKE_VARS -k || exit $? > # ... > - make test $EXPLICIT_MAKE_VARS || exit $? > > I have not tested whether this works on Travis-CI, but if it does, it > would be a nice improvement. I will take a look this week-end. I think the scripts are more maintainable outside of travis yaml files as separate build scripts, for two reasons: 1. One can just execute the script locally, you can't, AFAIK, do that with a travis yaml file. 2. The issue can be avoided as they afford more control. Some other projects I am a part of we only use script and after_failure. The bash scripts are set -e. I also used this approach for the KVM selinux test run. script: - ./.ci/travis.run after_failure: - cat build/test-suite.log We could adopt like what's above... > > Cheers, > Nicolas > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: travis: any reason we have keep going on make commands 2020-07-10 15:22 ` William Roberts @ 2020-07-11 16:56 ` Nicolas Iooss 2020-07-11 19:28 ` William Roberts 0 siblings, 1 reply; 7+ messages in thread From: Nicolas Iooss @ 2020-07-11 16:56 UTC (permalink / raw) To: William Roberts; +Cc: Paul Moore, SElinux list, Stephen Smalley On Fri, Jul 10, 2020 at 5:23 PM William Roberts <bill.c.roberts@gmail.com> wrote: > > On Fri, Jul 10, 2020 at 4:10 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > > > On Thu, Jul 9, 2020 at 8:00 PM Paul Moore <paul@paul-moore.com> wrote: > > > > > > On Thu, Jul 9, 2020 at 10:33 AM William Roberts > > > <bill.c.roberts@gmail.com> wrote: > > > > So Nicolas initially created our travis script in commit c9adfe2d2653 > > > > and has -k, or keep going, on the make commands. This causes make to > > > > plow ahead and bury the errors in the logs. Stephen noticed this the > > > > other day, and we have been chatting about it out of band and wanted > > > > to pull in the community. > > > > > > > > Are their compelling reasons for keeping this behavior? I am also > > > > concerned that we could get false positives on travis success results. > > > > > > In my opinion the whole point of automated testing is to catch > > > failures early and often. For that reason I would want the test to > > > fail and stop, both because I find it easier to identify the failure > > > that way and also because I'm not sure I would trust much of the > > > testing that occurred after an error condition. > > > > > Hi, > > There seems to be some confusion: > > > > * "make -k" does not stop the "make" command at the first error and > > allows seeing all the errors when there are several ones. In my humble > > opinion, it makes sense when compiling ("make all") and not when > > running tests ("make test"), and this is actually what is right now in > > Travis-CI. "make -k" returns a failure exit code when an error > > happens. > > Ahh I thought it returned to 0. Not sure why I assumed that. > > > > > * Travis-CI does not stop the job as soon as a sub-command fails. If I > > Depends on the stage: > https://docs.travis-ci.com/user/job-lifecycle/ > If before_install, install or before_script returns a non-zero exit > code, the build is errored and stops immediately. > If script returns a non-zero exit code, the build is failed, but > continues to run before being marked as failed. > > I put a false command in the script section and it kept plowing ahead > as you foretold. > > > understand correctly, this is what really bothers William, and I agree > > this is a behavior that can be improved. According to > > https://github.com/travis-ci/travis-ci/issues/1066, a possible > > solution could be to use "set -e", which could have unexpected > > side-effects in launched commands. It is possible to "emulate set -e" > > by adding exit statements, such as : > > > > - make install $EXPLICIT_MAKE_VARS -k || exit $? > > - make install-pywrap $EXPLICIT_MAKE_VARS -k || exit $? > > - make install-rubywrap $EXPLICIT_MAKE_VARS -k || exit $? > > # ... > > - make test $EXPLICIT_MAKE_VARS || exit $? > > > > I have not tested whether this works on Travis-CI, but if it does, it > > would be a nice improvement. I will take a look this week-end. > > I think the scripts are more maintainable outside of travis yaml files > as separate build scripts, > for two reasons: > 1. One can just execute the script locally, you can't, AFAIK, do that > with a travis yaml file. > 2. The issue can be avoided as they afford more control. Some other > projects I am a part of we only > use script and after_failure. The bash scripts are set -e. I also > used this approach for the KVM > selinux test run. > > script: > - ./.ci/travis.run > after_failure: > - cat build/test-suite.log > > We could adopt like what's above... I did not understand the part about using "cat build/test-suite.log", but otherwise the idea of putting the commands into a dedicated script file sounds good, as this allows more flexibility. In order to know which command failed (and fail as soon as a command fails), I suggest using "set -e -x" in these scripts. By the way, about the issue with "make", there is another thing from your initial message that I understood only after sending my first reply: a consequence of using "make -k" that can be considered undesirable is the fact that when an error happens, its message can be drowned among the flow of other messages. I disagree with removing "-k" because this would only show one error, instead of all the errors that occur during a build. Nevertheless it is possible to achieve the best of both alternatives by using constructions such as: if ! make install $EXPLICIT_MAKE_VARS -k ; then echo >&2 "Error in make install $EXPLICIT_MAKE_VARS:" make install $EXPLICIT_MAKE_VARS # This command shows one error, at the end of the build logs. exit 1 fi Would such constructions be helpful? Cheers, Nicolas ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: travis: any reason we have keep going on make commands 2020-07-11 16:56 ` Nicolas Iooss @ 2020-07-11 19:28 ` William Roberts 2020-07-13 19:01 ` William Roberts 0 siblings, 1 reply; 7+ messages in thread From: William Roberts @ 2020-07-11 19:28 UTC (permalink / raw) To: Nicolas Iooss; +Cc: Paul Moore, SElinux list, Stephen Smalley On Sat, Jul 11, 2020 at 11:57 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > On Fri, Jul 10, 2020 at 5:23 PM William Roberts > <bill.c.roberts@gmail.com> wrote: > > > > On Fri, Jul 10, 2020 at 4:10 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > > > > > On Thu, Jul 9, 2020 at 8:00 PM Paul Moore <paul@paul-moore.com> wrote: > > > > > > > > On Thu, Jul 9, 2020 at 10:33 AM William Roberts > > > > <bill.c.roberts@gmail.com> wrote: > > > > > So Nicolas initially created our travis script in commit c9adfe2d2653 > > > > > and has -k, or keep going, on the make commands. This causes make to > > > > > plow ahead and bury the errors in the logs. Stephen noticed this the > > > > > other day, and we have been chatting about it out of band and wanted > > > > > to pull in the community. > > > > > > > > > > Are their compelling reasons for keeping this behavior? I am also > > > > > concerned that we could get false positives on travis success results. > > > > > > > > In my opinion the whole point of automated testing is to catch > > > > failures early and often. For that reason I would want the test to > > > > fail and stop, both because I find it easier to identify the failure > > > > that way and also because I'm not sure I would trust much of the > > > > testing that occurred after an error condition. > > > > > > > Hi, > > > There seems to be some confusion: > > > > > > * "make -k" does not stop the "make" command at the first error and > > > allows seeing all the errors when there are several ones. In my humble > > > opinion, it makes sense when compiling ("make all") and not when > > > running tests ("make test"), and this is actually what is right now in > > > Travis-CI. "make -k" returns a failure exit code when an error > > > happens. > > > > Ahh I thought it returned to 0. Not sure why I assumed that. > > > > > > > > * Travis-CI does not stop the job as soon as a sub-command fails. If I > > > > Depends on the stage: > > https://docs.travis-ci.com/user/job-lifecycle/ > > If before_install, install or before_script returns a non-zero exit > > code, the build is errored and stops immediately. > > If script returns a non-zero exit code, the build is failed, but > > continues to run before being marked as failed. > > > > I put a false command in the script section and it kept plowing ahead > > as you foretold. > > > > > understand correctly, this is what really bothers William, and I agree > > > this is a behavior that can be improved. According to > > > https://github.com/travis-ci/travis-ci/issues/1066, a possible > > > solution could be to use "set -e", which could have unexpected > > > side-effects in launched commands. It is possible to "emulate set -e" > > > by adding exit statements, such as : > > > > > > - make install $EXPLICIT_MAKE_VARS -k || exit $? > > > - make install-pywrap $EXPLICIT_MAKE_VARS -k || exit $? > > > - make install-rubywrap $EXPLICIT_MAKE_VARS -k || exit $? > > > # ... > > > - make test $EXPLICIT_MAKE_VARS || exit $? > > > > > > I have not tested whether this works on Travis-CI, but if it does, it > > > would be a nice improvement. I will take a look this week-end. > > > > I think the scripts are more maintainable outside of travis yaml files > > as separate build scripts, > > for two reasons: > > 1. One can just execute the script locally, you can't, AFAIK, do that > > with a travis yaml file. > > 2. The issue can be avoided as they afford more control. Some other > > projects I am a part of we only > > use script and after_failure. The bash scripts are set -e. I also > > used this approach for the KVM > > selinux test run. > > > > script: > > - ./.ci/travis.run > > after_failure: > > - cat build/test-suite.log > > > > We could adopt like what's above... > > I did not understand the part about using "cat build/test-suite.log", > but otherwise the idea of putting the commands into a dedicated script Oh ignore that, that's a particular artifact of using automake's test log compiler for my particular project. You won't see that in stdout so on failure to see the details of each test run I have to do that. > file sounds good, as this allows more flexibility. In order to know > which command failed (and fail as soon as a command fails), I suggest > using "set -e -x" in these scripts. Yep, exactly. > > By the way, about the issue with "make", there is another thing from > your initial message that I understood only after sending my first > reply: a consequence of using "make -k" that can be considered > undesirable is the fact that when an error happens, its message can be > drowned among the flow of other messages. I disagree with removing > "-k" because this would only show one error, instead of all the errors > that occur during a build. Nevertheless it is possible to achieve the > best of both alternatives by using constructions such as: > > if ! make install $EXPLICIT_MAKE_VARS -k ; then > echo >&2 "Error in make install $EXPLICIT_MAKE_VARS:" > make install $EXPLICIT_MAKE_VARS # This command shows one error, > at the end of the build logs. > exit 1 > fi > > Would such constructions be helpful? I was thinking initially that -k caused make to return 0 not 1, and that -k was the root cause. That's not the case as you pointed out, so I think we can ignore that part. I think N errors in a make build are fine, so long as the rest of it doesn't attempt to run. > > Cheers, > Nicolas > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: travis: any reason we have keep going on make commands 2020-07-11 19:28 ` William Roberts @ 2020-07-13 19:01 ` William Roberts 0 siblings, 0 replies; 7+ messages in thread From: William Roberts @ 2020-07-13 19:01 UTC (permalink / raw) To: Nicolas Iooss; +Cc: Paul Moore, SElinux list, Stephen Smalley On Sat, Jul 11, 2020 at 2:28 PM William Roberts <bill.c.roberts@gmail.com> wrote: > > On Sat, Jul 11, 2020 at 11:57 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > > > On Fri, Jul 10, 2020 at 5:23 PM William Roberts > > <bill.c.roberts@gmail.com> wrote: > > > > > > On Fri, Jul 10, 2020 at 4:10 AM Nicolas Iooss <nicolas.iooss@m4x.org> wrote: > > > > > > > > On Thu, Jul 9, 2020 at 8:00 PM Paul Moore <paul@paul-moore.com> wrote: > > > > > > > > > > On Thu, Jul 9, 2020 at 10:33 AM William Roberts > > > > > <bill.c.roberts@gmail.com> wrote: > > > > > > So Nicolas initially created our travis script in commit c9adfe2d2653 > > > > > > and has -k, or keep going, on the make commands. This causes make to > > > > > > plow ahead and bury the errors in the logs. Stephen noticed this the > > > > > > other day, and we have been chatting about it out of band and wanted > > > > > > to pull in the community. > > > > > > > > > > > > Are their compelling reasons for keeping this behavior? I am also > > > > > > concerned that we could get false positives on travis success results. > > > > > > > > > > In my opinion the whole point of automated testing is to catch > > > > > failures early and often. For that reason I would want the test to > > > > > fail and stop, both because I find it easier to identify the failure > > > > > that way and also because I'm not sure I would trust much of the > > > > > testing that occurred after an error condition. > > > > > > > > > Hi, > > > > There seems to be some confusion: > > > > > > > > * "make -k" does not stop the "make" command at the first error and > > > > allows seeing all the errors when there are several ones. In my humble > > > > opinion, it makes sense when compiling ("make all") and not when > > > > running tests ("make test"), and this is actually what is right now in > > > > Travis-CI. "make -k" returns a failure exit code when an error > > > > happens. > > > > > > Ahh I thought it returned to 0. Not sure why I assumed that. > > > > > > > > > > > * Travis-CI does not stop the job as soon as a sub-command fails. If I > > > > > > Depends on the stage: > > > https://docs.travis-ci.com/user/job-lifecycle/ > > > If before_install, install or before_script returns a non-zero exit > > > code, the build is errored and stops immediately. > > > If script returns a non-zero exit code, the build is failed, but > > > continues to run before being marked as failed. > > > > > > I put a false command in the script section and it kept plowing ahead > > > as you foretold. > > > > > > > understand correctly, this is what really bothers William, and I agree > > > > this is a behavior that can be improved. According to > > > > https://github.com/travis-ci/travis-ci/issues/1066, a possible > > > > solution could be to use "set -e", which could have unexpected > > > > side-effects in launched commands. It is possible to "emulate set -e" > > > > by adding exit statements, such as : > > > > > > > > - make install $EXPLICIT_MAKE_VARS -k || exit $? > > > > - make install-pywrap $EXPLICIT_MAKE_VARS -k || exit $? > > > > - make install-rubywrap $EXPLICIT_MAKE_VARS -k || exit $? > > > > # ... > > > > - make test $EXPLICIT_MAKE_VARS || exit $? > > > > > > > > I have not tested whether this works on Travis-CI, but if it does, it > > > > would be a nice improvement. I will take a look this week-end. > > > > > > I think the scripts are more maintainable outside of travis yaml files > > > as separate build scripts, > > > for two reasons: > > > 1. One can just execute the script locally, you can't, AFAIK, do that > > > with a travis yaml file. > > > 2. The issue can be avoided as they afford more control. Some other > > > projects I am a part of we only > > > use script and after_failure. The bash scripts are set -e. I also > > > used this approach for the KVM > > > selinux test run. > > > > > > script: > > > - ./.ci/travis.run > > > after_failure: > > > - cat build/test-suite.log > > > > > > We could adopt like what's above... > > > > I did not understand the part about using "cat build/test-suite.log", > > but otherwise the idea of putting the commands into a dedicated script > > Oh ignore that, that's a particular artifact of using automake's test > log compiler > for my particular project. You won't see that in stdout so on failure to see > the details of each test run I have to do that. > > > file sounds good, as this allows more flexibility. In order to know > > which command failed (and fail as soon as a command fails), I suggest > > using "set -e -x" in these scripts. > > Yep, exactly. > > > > > By the way, about the issue with "make", there is another thing from > > your initial message that I understood only after sending my first > > reply: a consequence of using "make -k" that can be considered > > undesirable is the fact that when an error happens, its message can be > > drowned among the flow of other messages. I disagree with removing > > "-k" because this would only show one error, instead of all the errors > > that occur during a build. Nevertheless it is possible to achieve the > > best of both alternatives by using constructions such as: > > > > if ! make install $EXPLICIT_MAKE_VARS -k ; then > > echo >&2 "Error in make install $EXPLICIT_MAKE_VARS:" > > make install $EXPLICIT_MAKE_VARS # This command shows one error, > > at the end of the build logs. > > exit 1 > > fi > > > > Would such constructions be helpful? > > I was thinking initially that -k caused make to return 0 not 1, and > that -k was the > root cause. That's not the case as you pointed out, so I think we can > ignore that > part. I think N errors in a make build are fine, so long as the rest > of it doesn't > attempt to run. FYI I filed a bug on this: https://github.com/SELinuxProject/selinux/issues/256 So we don't lose this. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-07-13 19:01 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-09 14:33 travis: any reason we have keep going on make commands William Roberts 2020-07-09 18:00 ` Paul Moore 2020-07-10 9:08 ` Nicolas Iooss 2020-07-10 15:22 ` William Roberts 2020-07-11 16:56 ` Nicolas Iooss 2020-07-11 19:28 ` William Roberts 2020-07-13 19:01 ` William Roberts
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.