Re: [Automated-testing] KCIDB: Support one more test status

[Nikolai Kondrashov <Nikolai.Kondrashov@redhat.com>]

Hi Guillaume,

Thank you for the response.
I snippet some old stuff, and answered a bit, below.

On 5/5/23 14:39, Guillaume Tucker wrote:
> On 20/04/2023 18:37, Nikolai Kondrashov wrote:
>> Yes, the "ERROR" status is for when we actually got to executing the test (suite), and then it messed up, and e.g. crashed in the middle of execution. This is likely a problem for test maintainers to solve, and not for the CI people (although they would do well to keep an eye on such results too).
> 
> OK I think the documentation could be updated to provide a bit
> more context about what's expected from each status:
> 
>    https://kernelci.org/docs/kcidb/submitter_guide/#status
> 
> At the moment, it just says that ERROR means:
> 
>    "the test is faulty, the status of the tested code is unknown."
> 
> The word "test" here I think means the code written to run a test
> to you, but I can see how this might be interpreted differently
> and things that prevent the test from being run could slip in and
> be reported as errors.  Maybe adding something about who in
> principle is responsible for each error status would help as
> discussed in yesterday's meeting (e.g. ERROR is for the test
> authors, MISS is for the people in charge of the test
> infrastructure).

Yes, we could use better documentation, definitely. Have you taken a look at 
the PR in question? I refined the schema docs there a bit, do they make more 
sense now?

Here's the PR: https://github.com/kernelci/kcidb-io/pull/74

If that looks good, I'll update the Submitter Guide with this and go into a 
bit more detail, adding the table I used here to break down the responsibility.

>> Yes, as I also described above, if you care about which exact tests a test suite executes, and some tests in your previously-submitted plan didn't execute after all, then you would need to set their status to "MISS".
> 
> Right, even though I think that's a grey area between the test
> and the infrastructure.  Maybe no status would also work here.
> 

Yes, often it's hard to determine what's at fault, but when you can it's best 
to avoid alerting people who are not responsible for the problem (such as not 
notifying the test maintainer in case of CI failure).

>> But if you only care that this suite executes, and runs whatever tests it deems important, you could e.g. send an object for the whole test suite with its status missing before starting. Then send its subtests with status filled in with whatever you get as they execute, capping off with the final status of the whole suite after it's done. And if you failed to run the suite for whatever reason, then you could send the "MISS" status for it instead.
> 
> I think my main point was that we could just have ERROR with a
> broader scope and then something else like an error type or
> message to differentiate errors between the test, the
> infrastructure or anything specific to the CI system.  So from a
> user point of view, it's either a valid result i.e. PASS / FAIL /
> SKIP or something went wrong and it's ERROR.

Sure, some CI systems would only be able to report "ERROR", that's OK, and 
even if you try hard to report a "MISS" when you can do it, "ERROR" would also 
keep slipping in by mistake.

While we already have a lot of fields (and need more still), I think "MISS" 
fits the value space of existing test statuses quite well. After all, we 
already try to distinguish "ERROR" from "FAIL", splitting the test maintainer 
responsibility from the developer responsibility (when possible). We're just 
adding a finer gradation here, splitting "ERROR" again, into test maintainer 
and CI maintainer responsibilities.

> On the other hand, having MISS shouldn't really be a problem.  I
> can see why it could be very useful to have it in KCIDB for CKI's
> use-case at least.  So I'm still not personally convinced by this
> but I think it's fine to add it anyway as I don't see any
> blocking reason for not adding it either.

Thank you, Guillaume. I have now merged the PR and will proceed on finishing 
support for it in the notifications and dashboards. Right now the new schema 
version is not accepted by Production yet.

Nick