Hi,
I'd like to enquire if it's desired to let runtask exit with '1' if the task in question has a 'FAILED' outcome.
Maybe there was a conscious decision of not providing this feature and I'd like to hear about it.
I've joined Taskotron on phabricator. If there are no obvious resentments against such an idea, I'm happy to create a task and implement it.
Kind Regards,
Hi,
I'd like to enquire if it's desired to let runtask exit with '1' if the task in question has a 'FAILED' outcome.
Maybe there was a conscious decision of not providing this feature and I'd like to hear about it.
I've joined Taskotron on phabricator. If there are no obvious resentments against such an idea, I'm happy to create a task and implement it.
Kind Regards,
Róman Joost
Hey Róman,
thanks for joining the project and welcome :)
We have never really thought about the feature you're talking about. We currently return non-zero in case the execution fails - ctrl+c, an exception thrown. If we wanted to extend it to the FAILED task result, I see the following issues:
1. We have other outcomes defined at the moment, like NEEDS_INSPECTION: https://docs.qadevel.cloud.fedoraproject.org/libtaskotron/latest/library.htm... What should happen in that case?
2. Some tasks do not check just a single item. For example, depcheck and upgradepath check everything that is currently proposed for stable or testing updates. They can return a dozen test results. What should happen when there are mixed results?
3. At the moment, it's technically possible to leave out the 'resultsdb' directive the from task recipe. I cannot imagine a use case for this, because in development mode, this only reports to stdout, but it's possible if you need it. If we wanted to inspect task outcomes, we need to do it from TAP, therefore in the 'resultsdb' directive. It might be confusing to people that if you commented out that directive, runtask would stop returning "correct" return codes.
One further thought, instead of implementing this by default, we can also create a new directive or variable which will allow task creators to set the exit code, according to TAP or any other logic they want.
Why exactly do you need this? For local testing? runtask is just an execution wrapper, so you should be able to run your `./script some args` the same way as `runtask -i item -t type script.yml`. Except for some set up steps (like 'koji' or 'bodhi' directive), so if you use some of these, I can see a reason why you would want to run it through runtask.
Kamil
Dear Kamil,
thanks for your prompt reply.
On Tue, Mar 24, 2015 at 05:21:44AM -0400, Kamil Paral wrote:
Hi,
I'd like to enquire if it's desired to let runtask exit with '1' if the task in question has a 'FAILED' outcome.
Maybe there was a conscious decision of not providing this feature and I'd like to hear about it.
I've joined Taskotron on phabricator. If there are no obvious resentments against such an idea, I'm happy to create a task and implement it.
Kind Regards,
Róman Joost
Moved this up, since I have not explained my motivation in my initial post:
Why exactly do you need this? For local testing? runtask is just an execution wrapper, so you should be able to run your `./script some args` the same way as `runtask -i item -t type script.yml`. Except for some set up steps (like 'koji' or 'bodhi' directive), so if you use some of these, I can see a reason why you would want to run it through runtask.
That's exactly why I thought about runtask. To be more precise, we're currently working on rpmgrill. It ships it's own fetch-build script which is currently tied to Fedoras Koji.
IMHO rpmgrill shouldn't be concerned about where the builds come from. If I can use 'runtask' to fetch the builds, let rpmgrill handle the analysis and report back to bodhi or any other similar system that would be very beneficial. Furthermore that can run easily in a Jenkins and the exit status would provide a way for the CI to figure if the package is not shipping with nasty problems.
Hey Róman,
thanks for joining the project and welcome :)
We have never really thought about the feature you're talking about. We currently return non-zero in case the execution fails - ctrl+c, an exception thrown. If we wanted to extend it to the FAILED task result, I see the following issues:
- We have other outcomes defined at the moment, like NEEDS_INSPECTION:
https://docs.qadevel.cloud.fedoraproject.org/libtaskotron/latest/library.htm... What should happen in that case?
Theoretically it'll be an exit code 1 as well. What I don't know tho is, if it's practical to change it to 1. Perhaps it would mean that currently all packages which are under test would basically 'fail'. You might have a better insight here what the consequences of that change would be.
- Some tasks do not check just a single item. For example, depcheck and upgradepath check everything that is currently proposed for stable or testing updates. They can return a dozen test results. What should happen when there are mixed results?
I'd assume to default to the worst result. So if any one of them has a a FAILED state it means exit with 1. What I'm not sure about is if it's currently easy to accumulate the worst result case with the current code. Might be easier thought than done.
- At the moment, it's technically possible to leave out the 'resultsdb' directive the from task recipe. I cannot imagine a use case for this, because in development mode, this only reports to stdout, but it's possible if you need it. If we wanted to inspect task outcomes, we need to do it from TAP, therefore in the 'resultsdb' directive. It might be confusing to people that if you commented out that directive, runtask would stop returning "correct" return codes.
IMHO that's the beauty of Taskotron being decoupled from the execution and the result reporting. My main idea was just using libtaskotron for task execution and perhaps in the future to report to other aggregation systems inside Red Hat (the equivalent to Bodhi).
So I thought from a developer point of view, it allows to run build analysers against your package in your CI using Taskotron without the need to report the results back into a results database.
I hope I'm not twisting the intention of the whole Taskotron idea.
One further thought, instead of implementing this by default, we can also create a new directive or variable which will allow task creators to set the exit code, according to TAP or any other logic they want.
That sounds like a good idea as well. Based on that, what we could also do is, add an argument to runtask like:
--make-fatal
which means if the result of the runtask is not PASSED or INFO it'll exit with 1. Less flexible than your idea, but perhaps less work involved.
Kind Regards,
That's exactly why I thought about runtask. To be more precise, we're currently working on rpmgrill. It ships it's own fetch-build script which is currently tied to Fedoras Koji.
Just a note, IIRC you don't need just all the rpms, but also the build log for certain subtests. That's currently not provided by our koji directive, and I'm not sure if we'll want to add it or not. In such cases, you might need to implement some bits on your own (e.g. fetching the build log).
IMHO rpmgrill shouldn't be concerned about where the builds come from. If I can use 'runtask' to fetch the builds, let rpmgrill handle the analysis and report back to bodhi or any other similar system that would be very beneficial. Furthermore that can run easily in a Jenkins and the exit status would provide a way for the CI to figure if the package is not shipping with nasty problems.
I see, so you want to use the exit status as a super-simple way to distinguishing pass/fail in Jenkins, instead of providing some more advanced structure that the CI understands (junit, etc). I think that's a valid request.
- We have other outcomes defined at the moment, like NEEDS_INSPECTION:
https://docs.qadevel.cloud.fedoraproject.org/libtaskotron/latest/library.htm... What should happen in that case?
Theoretically it'll be an exit code 1 as well. What I don't know tho is, if it's practical to change it to 1. Perhaps it would mean that currently all packages which are under test would basically 'fail'. You might have a better insight here what the consequences of that change would be.
I think NEEDS_INSPECTION would also be considered as failed, and the exit code would be a predefined value (e.g. 100), so that we can still distinguish execution errors from a task failure.
- Some tasks do not check just a single item. For example, depcheck and
upgradepath check everything that is currently proposed for stable or testing updates. They can return a dozen test results. What should happen when there are mixed results?
I'd assume to default to the worst result. So if any one of them has a a FAILED state it means exit with 1. What I'm not sure about is if it's currently easy to accumulate the worst result case with the current code. Might be easier thought than done.
We had a long discussion with Josef about this, and our conclusion is that it should be the script author who is in business of deciding what the overall result is. The easiest way to implement this seems to be to let the author define one additional result in TAP (the very last result in TAP), according to any logic he or she needs, and we will consider this last result to be the decision of what exit code to return.
So, if you have just a single result in TAP, it's the one that is used for exit code decision. If you have multiple of them, the last one is used.
This means you will *have to* return at least a basic TAP from rpmgrill (there's a perl library for that), or from the wrapper around rpmgrill (e.g. converting its json to TAP). But TAP itself is extremely simple, so it shouldn't be a problem doing it even manually, i.e. in tests written in bash (we don't support this yet, but plan it in the future). This is the simplest TAP we can consume (I just discovered a bug here, but I'll fix it):
TAP version 13 1..1 not ok
And this is the simplest version that makes sense if you want to report to resultsdb:
TAP version 13 1..1 not ok --- item: htop-1.0.3-4.fc21 outcome: FAILED type: koji_build ...
- At the moment, it's technically possible to leave out the 'resultsdb'
directive the from task recipe. I cannot imagine a use case for this, because in development mode, this only reports to stdout, but it's possible if you need it. If we wanted to inspect task outcomes, we need to do it from TAP, therefore in the 'resultsdb' directive. It might be confusing to people that if you commented out that directive, runtask would stop returning "correct" return codes.
IMHO that's the beauty of Taskotron being decoupled from the execution and the result reporting. My main idea was just using libtaskotron for task execution and perhaps in the future to report to other aggregation systems inside Red Hat (the equivalent to Bodhi).
So I thought from a developer point of view, it allows to run build analysers against your package in your CI using Taskotron without the need to report the results back into a results database.
I hope I'm not twisting the intention of the whole Taskotron idea.
I was always thinking either about running tests in in the full taskotron environment (with resultsdb etc), or locally without any special needs (mainly for test development). But if you want to use libtaskotron as a runner in a different CI environment and we can satisfy your needs, I think that's great. And it will make sharing tests between Taskotron and internal Red Hat CI systems easier. No objections there.
One further thought, instead of implementing this by default, we can also create a new directive or variable which will allow task creators to set the exit code, according to TAP or any other logic they want.
That sounds like a good idea as well. Based on that, what we could also do is, add an argument to runtask like:
--make-fatalwhich means if the result of the runtask is not PASSED or INFO it'll exit with 1. Less flexible than your idea, but perhaps less work involved.
I like it. By using this cmdline option, you would trigger the behavior I described earlier (using the last TAP result as an exit code for the runner).
So, in summary, for your rpmgrill in Jenkins use case, you would run it with --make-fatal and you would make sure that the last result in TAP (or the only result in TAP, if you have just one) reflects the exit code you want runtask to return (FAILED and NEEDS_INSPECTION means non-zero). Does that sound reasonable (to everyone)?
Dear Kamil,
sorry for the delay.
On Wed, Mar 25, 2015 at 08:51:29AM -0400, Kamil Paral wrote:
That's exactly why I thought about runtask. To be more precise, we're currently working on rpmgrill. It ships it's own fetch-build script which is currently tied to Fedoras Koji.
Just a note, IIRC you don't need just all the rpms, but also the build log for certain subtests. That's currently not provided by our koji directive, and I'm not sure if we'll want to add it or not. In such cases, you might need to implement some bits on your own (e.g. fetching the build log).
Maybe talking about just builds is wrong, but perhaps a better term would be build artefacts. In order to accommodate scanners which not only check the build itself but logs producing the build, I'd think that logs given the scanners would make a good addition.
Perhaps better to be discussed separately tho.
IMHO rpmgrill shouldn't be concerned about where the builds come from. If I can use 'runtask' to fetch the builds, let rpmgrill handle the analysis and report back to bodhi or any other similar system that would be very beneficial. Furthermore that can run easily in a Jenkins and the exit status would provide a way for the CI to figure if the package is not shipping with nasty problems.
I see, so you want to use the exit status as a super-simple way to distinguishing pass/fail in Jenkins, instead of providing some more advanced structure that the CI understands (junit, etc). I think that's a valid request.
I rather thought to be consistent the way typical test runner behave. I also know that some of them allow you to configure the behaviour since there are instances you want to check PASS or FAIL based on the logs.
[...] NEEDS_INSPECTION case
I think NEEDS_INSPECTION would also be considered as failed, and the exit code would be a predefined value (e.g. 100), so that we can still distinguish execution errors from a task failure.
- Some tasks do not check just a single item. For example, depcheck and
upgradepath check everything that is currently proposed for stable or testing updates. They can return a dozen test results. What should happen when there are mixed results?
I'd assume to default to the worst result. [...]
We had a long discussion with Josef about this, and our conclusion is that it should be the script author who is in business of deciding what the overall result is. The easiest way to implement this seems to be to let the author define one additional result in TAP (the very last result in TAP), according to any logic he or she needs, and we will consider this last result to be the decision of what exit code to return.
Question: what if the script author does not provide this additional step? What would be the fall back?
So, if you have just a single result in TAP, it's the one that is used for exit code decision. If you have multiple of them, the last one is used.
This means you will *have to* return at least a basic TAP from rpmgrill (there's a perl library for that), or from the wrapper around rpmgrill (e.g. converting its json to TAP). But TAP itself is extremely simple, so it shouldn't be a problem doing it even manually, i.e. in tests written in bash (we don't support this yet, but plan it in the future). This is the simplest TAP we can consume (I just discovered a bug here, but I'll fix it):
TAP version 13 1..1 not ok
And this is the simplest version that makes sense if you want to report to resultsdb:
TAP version 13 1..1 not ok
item: htop-1.0.3-4.fc21 outcome: FAILED type: koji_build ...
Sounds good.
One further thought, instead of implementing this by default, we can also create a new directive or variable which will allow task creators to set the exit code, according to TAP or any other logic they want.
That sounds like a good idea as well. Based on that, what we could also do is, add an argument to runtask like:
--make-fatalwhich means if the result of the runtask is not PASSED or INFO it'll exit with 1. Less flexible than your idea, but perhaps less work involved.
I like it. By using this cmdline option, you would trigger the behavior I described earlier (using the last TAP result as an exit code for the runner).
So, in summary, for your rpmgrill in Jenkins use case, you would run it with --make-fatal and you would make sure that the last result in TAP (or the only result in TAP, if you have just one) reflects the exit code you want runtask to return (FAILED and NEEDS_INSPECTION means non-zero). Does that sound reasonable (to everyone)?
Sounds good to me. I only wonder if we simply fall back to the current state (exit 0) if the precondition (last tap result outcome) and --make-fatal is not given.
Kind Regards,
Sounds good to me. I only wonder if we simply fall back to the current state (exit 0) if the precondition (last tap result outcome) and --make-fatal is not given.
That was the idea. If you specify --make-fatal, we return exit code 0 or 100 based on the outcome of the last TAP entry. If you don't specify it, we keep the current behavior - 0 if task completed (regardless of outcome), non-zero if execution aborted for some reason.
I have reported a new ticket here: https://phab.qadevel.cloud.fedoraproject.org/T450
How quickly do you need to have this implemented?
qa-devel@lists.fedoraproject.org
-
Kamil Paral -
Róman Joost