Remove javascript in HtmlLogger

[David Müller (daveMueller)]

Description

Remove javascript in HtmlLoggers xslt template

Related issue

closes #4365

• [x ] I have ensured that there is a previously discussed and approved issue.

Not sure why build is failing. Also I'm having issues testing the HtmlLogger when compiling vstest locally. Any guidance on that would be helpful to completely verify the changes. Currently I only emulated the use of Html.xslt.

Those fails look unrelated to your change, maybe this is just flakiness. Let me know when you are done with your changes, and I can review and help you fix it the tests.

Testing html logger is probably easiest if you build via build.cmd, it will create nuget in "artifacts\packages\Debug\Shipping\Microsoft.TestPlatform.17.9.0-dev.nupkg" that you can unpack and run vstest.console from there with --logger:html

Vstest.console is in the package in this path: "tools\net462\Common7\IDE\Extensions\TestPlatform\vstest.console.exe"

so e.g.: & "S:\p\vstest2\artifacts\packages\Debug\Shipping\Microsoft.TestPlatform.17.9.0-dev\tools\net462\Common7\IDE\Extensions\TestPlatform\vstest.console.exe" --logger:html s:\t\mstest81\bin\Debug\net8.0\mstest81.dll

/azp run

Azure Pipelines successfully started running 1 pipeline(s).

re-run, fails now on acceptance:

You can also test that way. But you must always build.cmd before running the acceptance tests, we pick up all the runners from the produced nugets to ensure what we ship is working. So the workflow is a bit clunky.

Adding some unit tests for the content produced by the html logger is probably not a bad idea.

Thanks, I let you know once I adapted the tests.

I fixed the issue and added an integration test. I can also confirm now that the look and feel of the pure HTML report is identical to the javascript solution. I think it even improved a bit as the black arrows now indicate that the results can be collapsed. Just let me know if I should improve something.

It's perfect, thank you :)

Could it be that this change is responsible for this new stairs effect? 🤔 I'm pretty sure I've never seen that before!

Could it be that this change is responsible for this new stairs effect? 🤔 I'm pretty sure I've never seen that before!

Yes you are right, thanks for reporting 🙏 . Sorry I made a mistake in the xslt template, I'll fix this tomorrow.

Cédric Luthi (@0xced) I checked this again today and it seems this is not related to the changes in this PR. The issue already exists in v17.9.0-preview-23531-01 which is before these changes. I analyzed this a bit further and the issue doesn't exist in v17.7.2 Could be that a dependency update caused this because there is definitely no change in the xslt template between those two version.

Continuing the analyzis by inspecting the generated HTML, it looks like the transformation is loosing a closing </div> from the template...
I suggest to open another issue for this.

I have submitted #4754 with full reproduction steps. The "only" difference between the correct and the erroneous report is the SDK version (respectively 6.0.417 and 8.0.100).

Thanks for the analysis here Dave, it helped a lot. The issue was that with the new writer the settings from xslt were not copied, and it output a self-closing

tag when there was no content in error-info. This is a valid XML syntax but not valid HTML syntax.

Same issue solved on SO: https://stackoverflow.com/questions/71357056/c-sharp-xslt-transformer-uses-self-closing-html-tags-which-are-invalid

Fix indentation

This PR is merged, if this bothers you, please start a new PR with the fix.