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

XSLT-Transform seems to ignore <xsl:template match="/"> #678

Open
broetchenrackete36 opened this issue Aug 28, 2019 · 20 comments
Open

XSLT-Transform seems to ignore <xsl:template match="/"> #678

broetchenrackete36 opened this issue Aug 28, 2019 · 20 comments

Comments

@broetchenrackete36
Copy link

I am trying to generate an overview-HTML of the TestResult, but the xslt-transform is ignoring everything between <xsl:template match="/"> and </xsl:template>.

I am using a modified version of the html-summary.xslt from nunit-transforms and all the css is missing in the resulting html. Same when I use the unmodified version from github.

When I transform the xml manually (with an online-tool or VS) it works as expected, so the xslt should be fine.

NUnit3-ConsoleRunner Version: 3.10.0

@ChrisMaddock
Copy link
Member

ChrisMaddock commented Aug 28, 2019

I have absolutely no idea why that would be - although I'm not much of an xslt user myself.

Here's the code that does the transform. Any ideas?
https://github.com/nunit/nunit-console/blob/master/src/NUnitEngine/nunit.engine/Services/ResultWriters/XmlTransformResultWriter.cs

@CharliePoole
Copy link
Member

Could it relate to the fact that the code that applies the transform targets .NET 2.0 and uses the 2.0 libraries?

@ChrisMaddock ChrisMaddock self-assigned this Aug 29, 2019
@ChrisMaddock
Copy link
Member

Ok, this issue is caused by the fact that the XmlTransformResultWriter currently just gets the test-run XmlNode, rather than the XDocument. (Which I presume is different because it includes the declaration?) That's why the transform works on a output test result file, but doesn't work inside the console. It can be fixed simply with this small change:

                _transform.Transform(result.OwnerDocument, xmlWriter);

Having transforms that work the same both in-console and in the exported file seems like the desired behaviour, however I'm a little wary about changing the behaviour here this long after the release of Nunit 3.0. I think this is a slightly more niche case however, in that this specific xslt is affected because it tries to match the route.

Think I need to come back to this and test further. Any thoughts, anyone?

@ChrisMaddock
Copy link
Member

I've thought about it. I think this needs to change. And be advertised as potentially breaking. I'll check if any of the nunit-transforms will break with this change when I PR.

@broetchenrackete36
Copy link
Author

Thanks for your analysis. For the time beeing at least i can use your fix locally. Unfortunatly i don't have enough inside into nunit3 to test it further than just using the console-runner with my xslt.

@CharliePoole
Copy link
Member

@ChrisMaddock Have you considered just adding a second extension in the same assembly? It would only take a few lines of code and you could refactor so that the new one used the document while the old one still worked as before. Both extensions would be installed but the user would choose which one to use by providing a different format.

@broetchenrackete36
Copy link
Author

@ChrisMaddock @CharliePoole Sorry to bother you two, but are there any news regarding this issue?

@CharliePoole
Copy link
Member

@ChrisMaddock Should we treat this as a console problem or an nunit-transforms problem? If it once worked and the console changed at some point, then I'd say fix it in the console. Otherwise, maybe the xslt needs to change.

@CharliePoole
Copy link
Member

TLDR; After some review, I think the problem is the transform rather than nunit3-console -or maybe just the documentation for the html transforms. If possible, I should just fix the bad transforms. If not, it should either be a documented limitation OR the console can get a new option that works the way @ChrisMaddock has suggested.

HISTORY: the use of transforms in NUnit V2 pre-dated any use of HTML output, and therefore of CSS by the NUnit team. We viewed transforms as being used to change the output from one XML format to another XML format. It was a sort of innovation at the time that we also used a transform to create the console text output. (Yes, the actual text output of the console was originally created by transforming the XML file. That's why we traditionally show no output till the run is complete.) Further, with NUnit 2, there was no way to transform the XML except by using the output file.

By the time we did NUnit 3, console output of V2 was no longer being done via a transform. I removed that because I so disliked using XML and also so we could have output during the run. NUnit 3 went the same way.

I contributed the HTML transforms under the nunit-transforms project. They came from my own nunit-summary project, which creates transformed output from the saved result file. At the time I wrote them, I didn't know any other way to apply CSS other than through a match to the document root. I can research if there is a better way to do that. It looks to me as if all the transforms that don't use CSS - i.e. the non-HTML transforms still work correctly.

@broetchenrackete36
Copy link
Author

So I had a look at the xslt again, one way to solve it, as long as there is only one test-run per result-file, is to move the css section from <xsl:template match="/"> to <xsl:template match="test-run">. And even with multiple test-runs it is not a big problem, because the duplicated styles are simply ignored. If you decide to leave it as that, I'm fine with closing this issue.

@CharliePoole
Copy link
Member

NUnit will never produce more than one test-run element, so that's not an issue unless you concatenate files.

The use of match="/" was intended to deal with situations where there was no test-run, which I don't believe we need to worry about nowadays.

If @ChrisMaddock agrees I'll transfer this to the nunit-transforms project and fix it there.

@ChrisMaddock
Copy link
Member

I think the change should be made to the transforms either way - that sounds like an improvement to the transform as it is. 🙂 Charlie - I had a quick check, looks like this affects all the ex-nunit-summary transforms.

I'm a little less sure about whether to make a change to the console or not. I think the current behaviour, where transforming the TestResult.xml file 'by hand' has a different result to if you pass a transform into the console, is incorrect - however I'm a little wary about changing the behaviour so long after the NUnit 3.0 release - when people may have adapted to the functionality.

The other part of me thinks this is a straight up bug, and we should just fix it. Thoughts from @nunit/engine-team would be welcome!

@ChrisMaddock ChrisMaddock removed their assignment Oct 24, 2019
@CharliePoole
Copy link
Member

I think you're right. It makes more sense to give the writer a document rather than just a node. The question is "Will this break anything?" I'll leave that to you.

OTOH it seems as if the transforms should be improved as well. I'll create a separate issue for that.

@jnm2
Copy link
Collaborator

jnm2 commented Oct 27, 2019

It does sound like a bug, so I like the idea of fixing. I'm not sure how to help on the question of what sort of breaks could happen.

@CharliePoole
Copy link
Member

Key question for the transform part of the fix is to know whether there are still ways to produce a test result file that does not start with a test-run element. NUnitlite?

@CharliePoole
Copy link
Member

@broetchenrackete36 Does match="/*" work for you? I'm thinking of using that for all the transforms that generate CSS. In theory it should plug in the CSS for whatever the top level element is.

[The engine will always produce a test-run element, which solves the problem for runners that use the engine, but not for any that use the framework directly. Framework produces XML with a top level <test-suite type="Assembly"> element.]

@broetchenrackete36
Copy link
Author

@CharliePoole I'm not an XSLT-Guru and for some reason using match="/*" breaks my output completely... Even when using an external XSLT-Engine (Visual Studio XSLT)... CSS is generated correctly but the other templates fail to produce anything usefull..

@CharliePoole
Copy link
Member

Experimenting further, I found out the same thing. I ended up using match="/test-run". I'll merge that and you can comment on the nunit-transforms issue if it seems to need more tweaking.

@CharliePoole
Copy link
Member

@nunit/engine-team Is this still an issue?

@mikkelbu
Copy link
Member

At least we are still passing in a node in the code, so your comment in #678 (comment) is still relevant (but I know next to nothing about XSLT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants