Propagate result phase into the report plugins

[seberm]

Pull Request Checklist

• implement the feature
• write the documentation
• extend the test coverage
• update the specification
• adjust plugin docstring
• modify the json schema
• mention the version
• include a release note

This is a prerequisite for report plugins to support "subresults" aka phases.

Related to:

• Add support for test phases #2826
• Extend results schema to allow test phases #2933
• Introduce "phase result" for prepare/finish plugins to return [Step results #2] #2977

[psss]

Nice kick-off! Thanks. Looks good. Let's merge this soon to unblock experimeting in #3095 and with result presentation in individual report plugins.

[psss]

This one is really trivial, proposing to squeeze into 1.35.

[seberm]

Hello @happz @psss ,
I am still thinking about a tiny modification - I think we probably should create a new type for phase checks, e.g. PhaseCheckResult(CheckResult). This would allow us to easily differentiate between the result checks and the result phase checks, especially in the report plugins.

Do you agree?

[happz]

Hello @happz @psss , I am still thinking about a tiny modification - I think we probably should create a new type for phase checks, e.g. PhaseCheckResult(CheckResult).

What would be the difference between CheckResult and the new PhaseCheckResult class? The existing classes usually add new fields on top of their base class: CheckResult adds event, Result adds context and serial_number and so on, so what new fields would PhaseCheckResult carry that Check lacks?

This would allow us to easily differentiate between the result checks and the result phase checks, especially in the report plugins.

Test check results live in the check list, and phase check results live in the phases[N].check list. The structure, the fields describing the check outcome are the same, the position provides the context.

Do you agree?

Not as of now. Can you demonstrate in what way would different classes improve the code, e.g. making it simpler, easier to read and maintain, or easy to check for linters?

[seberm]

The existing classes usually add new fields on top of their base class: CheckResult adds event, Result adds context and serial_number and so on, so what new fields would PhaseCheckResult carry that Check lacks?

It would carry no new fields, just a type.

Not as of now. Can you demonstrate in what way would different classes improve the code, e.g. making it simpler, easier to read and maintain, or easy to check for linters?

It would allow me to easily differentiate between the result types in display_outcome(result: BaseResult) -> None and use this method to show all kinds of results. It's just an early draft:

• d10ebe3#diff-f9ad5f6605f10698f0208842ba0e42ee7bf1508151827baf78c3660b611e4457R84

If you still think this does not make sense, I will try to find a different solution.

[happz]

The existing classes usually add new fields on top of their base class: CheckResult adds event, Result adds context and serial_number and so on, so what new fields would PhaseCheckResult carry that Check lacks?

It would carry no new fields, just a type.

Ack.

Not as of now. Can you demonstrate in what way would different classes improve the code, e.g. making it simpler, easier to read and maintain, or easy to check for linters?

It would allow me to easily differentiate between the result types in display_outcome(result: BaseResult) -> None and use this method to show all kinds of results. It's just an early draft:

• d10ebe3#diff-f9ad5f6605f10698f0208842ba0e42ee7bf1508151827baf78c3660b611e4457R84

I see, makes sense. Let's try it then and see how it goes.

[thrix]

LGTM

[happz]

Apparently unrelated failures in tests, going to merge this one and we'll see.