report: support multiple results reported in one test #2236
Conversation
Sub-result is important for testsuite which consists of many sub testcases. Without sub-result reported, it is very hard to analysis the failures. Signed-off-by: Murphy Zhou <jencce.kernel@gmail.com>
| _result = Result( | ||
| name=test.name, | ||
| classname=test.name, |
There was a problem hiding this comment.
I see there's only one use of classname, it's set to test.name, is it the correct move? I'm not very fluent in junit, can you help me understand what's the purpose of classname field? I'm slightly worried it's a junit feature leaking into a generic Result class, and maybe we could find a better name for the attribute, because, well, Result.classname is surprisingly not holding a class name :)
There was a problem hiding this comment.
Thanks for reviewing!
While junit format origins from Java, classname is refer to a test class which could contain a few steps.
If there is only one step, set "classname" to None and use "name" to identify the result is enough.
If there are multiple sub-results within one test, like steps in a Java class, we need to use "classname" to id the testcase, and use "name" to id the steps.
I see your points of class name :) Python class :)
OK, I'll try to find a better way.
There was a problem hiding this comment.
I see, thank you. It seems to me tmt has no use for classname as you described it, now, tmt's internal execute plugin does not recognize classes like pytest or Java frameworks - I guess if we ever get a pytest framework support, this feature would be needed there as well.
Having support for subresults makes sense to me, that's sure. First, I'd definitely keep name around, your patch is dropping it, replacing it with classname - name represents a test name as known to tmt, the one defined in fmf data, that's a crucial piece of information and I don't see it going away. New fields are fine, dropping name not so much :)
Second, right now, there's just one tmt test, and with custom results, https://tmt.readthedocs.io/en/stable/spec/tests.html#result, a test can generate "virtual" results nested under the test name, e.g. test /tests/foo can produce results with name set to /tests/foo/bar and /tests/foo/baz. That's similar to your subresults, but without any formal field for the structure. subresult field looks like a very good addition then, we can use it already for these "virtual" custom results - name would be set as it is now, plus we would set subresult to whatever the custom result brings in. This would be worth its own PR, BTW, it'd be a useful feature on its own.
But, I'm not sure the subresult concept in this sense fits the test/class/method relationship. I suppose that under a single tmt test, several Python/Java classes can be executed, each with multiple test methods, giving us plenty of (class, method) tuples. I would maybe try to add classname and methodname fields right away, but I'm not so sure we should fill them with values that are not classname or methodname, e.g. test.name. I'm not very fond of encoding multiple pieces of info and a single field, if we wish to store classname and methodname, if recognized by the test, then let's have two explicit fields for it :) It would be excellent if we could set these two fields via tmt-report-result explicitly, not masking them as a subresult or test name. In other words, adding fields rather than changing the semantics of existing ones. What do you think?
|
Is this still relevant. Perhaps covered by #2826 ? |
This have been covered, by result:restraint. Thanks! Closing. |
Sub-result is important for testsuite which consists of many sub testcases. Without sub-result reported, it is very hard to analysis the failures.