Project

General

Profile

Actions
Copy link

action #75388

open

coordination #102906: [saga][epic] Increased stability of tests with less "known failures", known incompletes handled automatically within openQA

coordination #102909: [epic] Prevent more incompletes already within os-autoinst or openQA

Explicit error feedback to test reviewers on wrong test API usage

Added by okurz about 4 years ago. Updated about 3 years ago.

Status:
Workable
Priority:
Low
Assignee:
-
Category:
Feature requests
Target version:
QA (public) - future
Start date:
Due date:
% Done:

0%

Estimated time:

Description

Motivation

In cases like #73231 there is a "die" message pointing to what unexpected situation was encountered but it is not clear to unexperienced test reviewers who needs to act on this, if it is a test maintainer or an instance admin or openQA developer. We should provide clear error feedback on wrong test API usage and distinguish from other error cases

Acceptance criteria

  • AC1: The "reason" field in openQA jobs makes it clear to readers if the test API was used in a wrong way, e.g. as in the case of #73231
  • AC2: os-autoinst contribution documentation explains how wrong test API usage can be distinguished from other error cases

Suggestions

  • Research existing "die" messages in os-autoinst
  • Explore how these die messages are displayed in openQA
  • Find a way to distinguish, e.g. a clear prefix in os-autoinst or exception vs. generic "die", custom die handler, etc.
  • Extend os-autoinst contribution documentation with hints or guidelines what to use in which case

Related issues 1 (0 open — 1 closed)

Copied from openQA Project (public) - action #73231: [microos]job incompletes with auto_review:"backend died: Virtio terminal and svirt serial terminal do not support send_key. Use"Resolvedggardet_arm2020-10-12

Actions
Actions
Copy link
 #1

Updated by okurz about 4 years ago

  • Copied from action #73231: [microos]job incompletes with auto_review:"backend died: Virtio terminal and svirt serial terminal do not support send_key. Use" added
Actions
Copy link
 #2

Updated by okurz about 4 years ago

  • Parent task set to #62420
Actions
Copy link
 #3

Updated by Xiaojing_liu about 4 years ago

  • Status changed from Workable to In Progress
  • Assignee set to Xiaojing_liu
Actions
Copy link
 #4

Updated by Xiaojing_liu about 4 years ago

I researched the existing 'die' and 'croak' in os-autoinst. I only find 2 places are about the test API is used in the wrong way:

  1. in consoles/serial_screen.pm, sub send_key, sub hold_key and so on.
  2. in testapi.pm, sub validate_script_output checks the input parameters and say croak "Invalid use of validate_script_output(), second arg must be a coderef or regexp"; And 1 is about #73231, the job will be treated as incomplete, the reason is backend die. And about 2, the job will be shown as failed, so the reason is shown in the job details. # Test died: Invalid use of validate_script_output(), second ...

I am not sure if I miss something or I am in the right direction.

About Suggestions 3, and 4:
a. Adding prefix in die message seems is the easiest way, is it enough?
b. Do we need to deal with the job result? for example if the reason matches the prefix, the job status is still incomplete? or give more details? such as a link to documentation?

Actions
Copy link
 #5

Updated by livdywan about 4 years ago

Xiaojing_liu wrote:

About Suggestions 3, and 4:
a. Adding prefix in die message seems is the easiest way, is it enough?
b. Do we need to deal with the job result? for example if the reason matches the prefix, the job status is still incomplete? or give more details? such as a link to documentation?

Do you think we can have a consistent prefix and job state? For example always make it Invalid API usage: validate_script_output and mark the job as Incomplete?

Actions
Copy link
 #6

Updated by Xiaojing_liu about 4 years ago

cdywan wrote:

Xiaojing_liu wrote:

About Suggestions 3, and 4:
a. Adding prefix in die message seems is the easiest way, is it enough?
b. Do we need to deal with the job result? for example if the reason matches the prefix, the job status is still incomplete? or give more details? such as a link to documentation?

Do you think we can have a consistent prefix and job state? For example always make it Invalid API usage: validate_script_output and mark the job as Incomplete?

Yes, I think we can have a prefix in scenario #73231. And the job is already Incomplete. Do we need to show backend died in the reason? For now, the reason is backend died: Virtio terminal and svirt serial terminal do not support send_key.

@okurz WDYT? do I miss something according to your suggestions on this ticket?

Actions
Copy link
 #7

Updated by Xiaojing_liu about 4 years ago

  • Due date set to 2020-11-13
Actions
Copy link
 #8

Updated by okurz about 4 years ago

  • Description updated (diff)

Xiaojing_liu wrote:

Yes, I think we can have a prefix in scenario #73231. And the job is already Incomplete. Do we need to show backend died in the reason? For now, the reason is backend died: Virtio terminal and svirt serial terminal do not support send_key.

We should not show "backend died" in the reason in case of wrong test API usage as I can imagine that "backend died" is understood as "something crashed". But it should be clear that the test writers need to act. We do not need to care or change the job state or result at this point. But if you think it is easier to follow the approach as in sub validate_script_output then try that.

@okurz WDYT? do I miss something according to your suggestions on this ticket?

Well, only the "contribution documentation" but that can also be treated separately.

Actions
Copy link
 #10

Updated by Xiaojing_liu about 4 years ago

  • Due date changed from 2020-11-13 to 2020-11-20
Actions
Copy link
 #11

Updated by okurz about 4 years ago

Hi, I suggest to update the due-date always with a comment. Our target should be to not exceed the mean cycle time which is currently at 14 days. Hence a due date of tomorrow would make sense. Considering that this often involves feedback from production and hence needs a merged PR that is deployed this is unlikely to be doable. Do you see a possibility to split this ticket? For learning purposes I think it would be also ok, maybe even beneficial, to unassign again and start a different ticket.

Actions
Copy link
 #12

Updated by Xiaojing_liu about 4 years ago

  • Due date deleted (2020-11-20)
  • Status changed from In Progress to Workable
  • Assignee deleted (Xiaojing_liu)

After getting the feedback from @cdywan and @okurz, I did not work on this ticket since last Wednesday.
I just had a thought about it:

  1. could we define a hash or array to store this special error message?
  2. Following 1, then we could modify the code die_handler in https://github.com/os-autoinst/os-autoinst/blob/master/backend/baseclass.pm#L92 to show these message as test died or something else not backend die. Not sure if I am in the right way. Since this ticket's due date, I unassign it.
Actions
Copy link
 #13

Updated by okurz almost 4 years ago

  • Target version changed from Ready to future
Actions
Copy link
 #14

Updated by okurz about 3 years ago

  • Parent task changed from #62420 to #102909
Actions
Copy link

Also available in: Atom PDF