Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Finalize XML Format for Test Results #254

Closed
CharliePoole opened this issue Sep 30, 2014 · 13 comments · Fixed by #886
Closed

Finalize XML Format for Test Results #254

CharliePoole opened this issue Sep 30, 2014 · 13 comments · Fixed by #886

Comments

@CharliePoole
Copy link
Contributor

Get comments from the community.

Specific issues:

  • Document the schema
  • How to record result state: two attributes or one?
@yetibrain
Copy link

Maybe we should not use attributes at all. If already now we are not sure whether to use one or two, we better use child tags. If a test has an attribute result="Success" it's senseless to additionally state executed="True". If it has a result, it must have been executed. Maybe something like this:

<test-case name="T19" description="Test T19" time="0.098" asserts="1">
<states>
<state name="result">Success</state>
</states>
</test-case>
<test-case name="T20" description="Test T20" time="0.098" asserts="1">
<states>
<state name="executed">False</state>
<state name="reason">Ignored</state>
</states>
</test-case>

@CharliePoole
Copy link
Contributor Author

We don't actually have an executed="True" attribute in the NUnit 3.0 XML
format. It is in NUnit 2.x, where we retained it for backward compatibility.

I made the notation about "one label or two" to remind myself to write it
up more thoroughly, so here goes:

An NUnit 3.0 ResultState has two components, called Status and Label.
Status is an enum with four values: Passed, Failed, Skipped or
Inconclusive. Label is a string and can have any value at all. Currently,
the XML uses separate attributes result and label for the two fields.

The options I was considering, regarding two versus one attributes were:

  1. Keep it as it is now, where you may have to examine two attributes to
    figure out what happened to a test.
  2. Make result be a single field, using the format Status:Label, which
    happens to be what ResultState.ToString() puts out.

There are a discrete set of values possible for ResultState internally, but
I anticipate allowing users to create custom states, which would use the
same enum but a different string label. The current possible internal
values of a ResultState, using the ':' notation are:

On Mon, Oct 6, 2014 at 8:29 AM, Peter Brightman notifications@github.com
wrote:

Maybe we should not use attributes at all. If already now we are not sure
whether to use one or two, we better use child tags. If a test has an
attribute result="Success" it's senseless to additionally state
executed="True". If it has a result, it must have been executed. Maybe
something like this:

Success False Ignored


Reply to this email directly or view it on GitHub
#254 (comment).

@CharliePoole
Copy link
Contributor Author

We don't actually have an executed="True" attribute in the NUnit 3.0 XML
format. It is in NUnit 2.x, where we retained it for backward compatibility.

I made the notation about "one label or two" to remind myself to write it
up more thoroughly, so here goes:

An NUnit 3.0 ResultState has two components, called Status and Label.
Status is an enum with four values: Passed, Failed, Skipped or
Inconclusive. Label is a string and can have any value at all. Currently,
the XML uses separate attributes result and label for the two fields.

The options I was considering, regarding two versus one attributes were:

  1. Keep it as it is now, where you may have to examine two attributes to
    figure out what happened to a test.
  2. Make result be a single field, using the format Status:Label, which
    happens to be what ResultState.ToString() puts out.

There are a discrete set of values possible for ResultState internally, but
I anticipate allowing users to create custom states, which would use the
same enum but a different string label. The current possible internal
values of a ResultState, using the combined syntax would be:
Passed (label is empty)
Failed
Failed:Error
Failed:Cancelled
Failed:Child (suites only)
Failed:SetUp "
Failed:TearDown "
Inconclusive
Skipped
Skipped:Ignored
Skipped:Invalid

For a more OO approach, having a single result field using the colon format
seems to make sense.

However, I suspect it's easier to find everything that failed, for whatever
reason, if we have the two fields.

I'd especially like opinions from folks who actually like to work with XML,
since they are bound to be different from mine. :-)

Charlie

On Mon, Oct 6, 2014 at 8:29 AM, Peter Brightman notifications@github.com
wrote:

Maybe we should not use attributes at all. If already now we are not sure
whether to use one or two, we better use child tags. If a test has an
attribute result="Success" it's senseless to additionally state
executed="True". If it has a result, it must have been executed. Maybe
something like this:

Success False Ignored


Reply to this email directly or view it on GitHub
#254 (comment).

@yetibrain
Copy link

So "executed" as well as "success" is no more part of 3.0, right?
I prefer two attributes.
Well i deal with xml along with xslt to create html result pages for the developers. Xslt of course uses xpath expressions. It is usually easier to have a single value in some attribute because otherwise you have to deal with substring-before/substring-after functions in order to catchup the right or left portion next to the colon. See also here: http://stackoverflow.com/questions/333249/how-to-apply-the-xpath-function-substring-after

@CharliePoole
Copy link
Contributor Author

OK, I take your point. So, if we follow that direction, each test will have
a result attribute with "Failed", "Passed", "Inconclusive" or "Skipped".
There will be an optional label attribute, which can have any value. For
example, "Ignored" for tests that were skipped because they have been
ignored.

Because of some ongoing work, not yet merged, there will also be an
optional site attribute, with a fixed set of values, probably: "SetUp",
"TearDown", "Child", "Parent".

Selecting all failed tests will simply use the result attribute, but
selecting something like Ignored precisely will involve looking at two
attributes... you can do a pretty good selection just looking at label but
then you are assuming that no label is used in common for two different
results.

Does that make sense?

On Tue, Oct 7, 2014 at 1:49 AM, Peter Brightman notifications@github.com
wrote:

So "executed" as well as "success" is no more part of 3.0, right?
I prefer two attributes.
Well i deal with xml along with xslt to create html result pages for the
developers. Xslt of course uses xpath expressions. It is usually easier to
have a single value in some attribute because otherwise you have to deal
with substring-before/substring-after functions in order to catchup the
right or left portion next to the colon. See also here:
http://stackoverflow.com/questions/333249/how-to-apply-the-xpath-function-substring-after


Reply to this email directly or view it on GitHub
#254 (comment).

@yetibrain
Copy link

Yes, it does make sense. As said before, to KIS it's better to avoid extracting substrings on values that indeed hold two values, separated by a colon. To test for an absent attribute is also easy by using the not keyword eg. "test-case/[@Result='Failed']][not(@Label)] finds all failed test-cases that have no label attribute. Of course xpath can be used with the label attribute in first case, but if a value for attribute label can occurr on different values of result, in this case of course both attributes have to be examined. And if there is a third attribute called site (as you have mentioned) it has to be considered within the xpath expression as well. Of course values of attributes can be ORed together in a predicate so something like "test-case/[@Result='Inconclusive' or @Result='Skipped']] can also make sense, depending on the report desired. If i want to know how many TearDowns have failed i'd use "count(./test-case[@Result='Failed'][@site='TearDown'])". We should fulfill the SRP, an attribute classifies a certain thing e. g. resultstate. A label should use a separate attribute, a site classification too.

@CharliePoole
Copy link
Contributor Author

Pragmatically speaking, that makes sense and I'm thinking its the way to
go. I don't agree with your last statement though. In fact, we're using
three attribute to represent one thing, a ResultState.

But it sounds like three attributes will work better.
On Oct 8, 2014 5:29 AM, "Peter Brightman" notifications@github.com wrote:

Yes, it does make sense. As said before, to KIS it's better to avoid
extracting substrings on values that indeed hold two values, separated by a
colon. To test for an absent attribute is also easy by using the not
keyword eg. "test-case/[@Result https://github.com/result
='Failed']][not(@Label https://github.com/label)] finds all failed
test-cases that have no label attribute. Of course xpath can be used with
the label attribute in first case, but if a value for attribute label can
occurr on different values of result, in this case of course both
attributes have to be examined. And if there is a third attribute called
site (as you have mentioned) it has to be considered within the xpath
expression as well. Of course values of attributes can be ORed together in
a predicate so something like "test-case/[@Result
https://github.com/result='Inconclusive' or @Result='Skipped']] can
also make sense, depending on the report desired. If i want to know how
many TearDowns have failed i'd use "count(./test-case[@Result
https://github.com/result='Failed'][@site='TearDown'])". We should
fulfill the SRP, an attribute classifies a certain thing e. g. resultstate.
A label should use a separate attribute, a site classification too.


Reply to this email directly or view it on GitHub
#254 (comment).

@CharliePoole
Copy link
Contributor Author

I'm working on documenting the current state of the XML format. See https://github.com/nunit/nunit/wiki/Test-Result-XML-Format

I see one issue with the <environment> element.

It shows up as a child of the <test-run> element, so there is only one for the entire run. However, a few of the attributes will vary when the test is run in another process. For example, the runtime version under which the test is running may not be the same one used for nunit itself and two different assemblies could be running under two different runtime versions. Should we move some of these values to the assembly level?

Additionally, if we once add a feature of running tests in another machine, many more of these values could potentially vary.

Ideas about how to deal with this are sought!

@CharliePoole
Copy link
Contributor Author

The description of the result file is complete at https://github.com/nunit/nunit/wiki/Test-Result-XML-Format and needs a review.

@yetibrain @rprouse Would you take a look?

I'm planning to change the text content of the output, message and stack-trace elements to CDATA sections. For attributes that have values provided or influenced by the user, I'm going to add code to replace any invalid XML characters, which will deal with issue #850 as well.

@ravichandranjv
Copy link

Hi Charlie,

I see one issue with the <environment> element.

As you had suggested, moving some of the attributes to the "assembly" makes more sense as 'runtime versions' make more sense with the <assembly> than with the <environment> of the running "nunit-version".

I do not want to suggest anything that may cause a rework but uncluttering the XML format more, will increase the readability, cleanliness of the resultant XML file, and may also help refactor the design.

Please ignore these below suggestions if they mean reworking the code and not refactoring the design.

SRP and DRY

  1. Since the test-cases are maintaining the count of asserts and this conforms to the single responsibility principle as well, why have the attribute, asserts at the test-suite level?
  2. The attribute total seems to maintain the same information as testcasecount in a test suite.

The output for your ready reference:

<test-suite type="Assembly" id="0-1003" name="test1.dll" fullname="E:\nunit3_tests\test1.dll" runstate="Runnable" testcasecount="2" result="Passed" start-time="2015-10-07 13:30:06Z" end-time="2015-10-07 13:30:07Z" duration="0.040075" total="2" passed="2" failed="0" inconclusive="0" skipped="0" asserts="0">

as does the attribute total at the root level.

The output for your ready reference:
<test-run id="2" testcasecount="2" result="Passed" total="2" passed="2" failed="0" inconclusive="0" skipped="0" asserts="0" start-time="2015-10-07 13:30:06Z" end-time="2015-10-07 13:30:07Z" duration="0.393900">

Instead of testcasecount appearing as an attribute of <test-run>, would it not make more sense to have testsuitecount as an attribute of <test-run>, as a test-case is already contained within test-suite?

@CharliePoole
Copy link
Contributor Author

Please let me clarify one thing: all changes to the format are programming changes. We have been producing XML output from NUnit 3.0 alphas and betas for a number of years. That's how we work. However, code is not steel and concrete. It can be changed if there is a reason to change it.

For your specific comments:

The asserts on a suite are equal to the number of asserts executed in all the test cases plus any asserts executed in the suite's OneTimeSetUp.

The testcasecount is the number of test cases found in a suite or assembly while the total is the number of test cases selected for execution. If no filter is used, then the values are the same, but they can easily be different.

I added info to the document explaining these points.

@CharliePoole
Copy link
Contributor Author

This comment will serve as a ToDo list of changes for completing the issue. I'll edit it as we go.

  1. Move some of the environment information to the assembly level.
  2. Add a filter element to the test-run, which will essentially be a copy of the filter that was passed in.

@CharliePoole
Copy link
Contributor Author

There will need to be future work on the environment element. I created issue #887 for that.

johnmwright pushed a commit to johnmwright/nunit that referenced this issue Oct 28, 2019
Executor now doesnt warn on missing RANDOM_SEED file  nunit#253
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants