-
Notifications
You must be signed in to change notification settings - Fork 6
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
Use JSON report data instead of single-file html #26
Comments
It's open source, they can adapt their solution, or our solution. Doesn't matter to me. If they need something else, contribute it :)
And the solution of separate files is imo not user friendly. User friendlyness > "HTML way of working". Also single-file html reports as output for tools isn't that uncommon imo.
Nothing is forced, this is one option. Lazy loading can be a new html report feature instead of replacing the old one. In fact I would prefer it as a new reporter so you still have the option of just generating one self-containing HTML file.
Because that's not discoverable, and because I wanted to contribute something that would spread awareness of mutation-testing-elements.
We can also support the json report for this.
I doubt it, but we'll cross that bridge when we come it it. Microsoft could decide to stop with azure devops as well 🤷♀️
Widely used so not worried about that. We also don't use any fancy features.
We could publish a html-file publisher, but I intend for this extension to have specific features related to mutation testing later. As we discussed before I plan to also support JSON reports by loading it in mutation-testing-elements. At that point we could add advanced features like pull request annotation. |
By choosing the json publishing you will force other frameworks to have a json reporter. Either way the frameworks will have to fit our reporting standard. I can imagine some frameworks would not like to dirty their reporters by adding a json reporter. And I would like to remind you that we offer this as a service to other frameworks. But that doesn't mean we have to adapt to all other frameworks. If they want to use our publisher they should adapt to our standards. I think it should not be the other way around. And if they would like to see a change. As Rouke said, it's open source so they can work on it themselves.
I second Roukes opinion here. Singe file outputs are way more user friendly than multi file outputs. Users don't care that the code is ugly (not html way of working) in the report. As long as it works and can be easily used and shared. |
Regarding this:
I recently spent a lot of time working as a 'user' to join multiple Stryker runs into one result view on a .NET projects with lots of solutions. I did so by running Stryker on each solution by itself and then joining the json result files. After that I was done (and happy as well, took quite some time!). I don't like that there would be another step of injecting the json file into the HTML file. One extra consideration is having that clear separation between 'data' (the json) and the 'UI' (the HTML): I like that a lot: the data can be used anywhere (extra calculations somewhere else?, version tracking?) while the report stays the same. |
Let me rephrase. I don't think, in the default case of using a html report generated by stryker, having to work with multiple files is the most user friendly. Of course if you have other requirements you don't want to be forced to use our html reporting solution, which is why the json reporter is also available. Nevertheless, if you select the html reporter in stryker-net, it is my opinion that it is most user friendly to get one single file report that can be shared, saved, moved around without risk of breaking links to other files which makes the html report non-functional.
As a developer this can be nice to have, which is why we have a json reporter.
Json reporter :) |
Wait, let's take a step back: the json reporter creates only the json file, correct? |
@rajbos Yes, they both output the same json. But the html reporter adds some html and js to display it. |
In stryker net the json reporter writes a json file to disk, the html reporter writes a self-containing html file to disk. In strykerjs and stryker4s the html reporter writes three files to disk. A js file containing the mutation-testing-elements html elements definition, a json file containing the json report and a html file that loads the Javascript, and json and combines them by placing the mutation-testing-elements report element in the DOM with the json file as source. That difference is why this extension can only support the stryker net html report currently, until we support uploading the json report and loading that report in a mutation-testing-elements bundled with this extension. |
Thanks for the clarification :-) |
Ok, thanks @Mobrockers for your explanation so far. It's clear you feel strongly about this, so feel free to close this issue. Some responses to your feedback:
I see the HTML reporter and azure-devops-mutationreport-publisher as two separate entities. I might want to have my own Stryker logo in my HTML report, but I would expect no logo (or a generic one) in the azure-devops-mutationreport-display tool. (I know we disagree here, no need to point it out).
You want me to open this issue?
Widely used is not a guarantee, especially in the JS world. Believe me, I know 😔. Not using any fancy features is a good start (although loading it via a data url might be considered "fancy", I don't have enough experience to know this). There is also the issue of making sure you're dependencies are up-to-date with regards to security findings. If you need help maintaining the dependencies, let me know. |
Sure not having a logo might be acceptable, but do you also expect that the report behaves (possibly) completely different from what you get native from your tool, because for example a mutation testing framework might support our schema but has a completely different implementation of the html report, instead of mutation-testing-elements?
This issue already contains most of the discussion about a json only report so keeping this issue open as a feature request seems fine to me, feel free to open a new issue for it if you think there's too much clutter due to the discussion.
I don't think the data url is a problem, I also don't think it's required to get this working. I should be able to just use the srcDoc attribute I believe but I had some issues with it and someone suggested data url so I used that instead. I enabled dependabot but I'm not sure what all the options mean. If there are any options that should be enabled or if there are other dependency management things we should consider please let me know or create a new issue for it. |
The mutationreport publisher right now works by publishing the HTML file and loading that in an IFrame (iframe in iframe), instead of publishing the JSON of the mutation testing report itself.
I have only communicate my dissatisfaction verbally, and did a poor job at it. Sorry for that. Let me try to convince you with arguments in this issue.
My proposal is to create a new publisher that works with the report JSON and rebrand this one as a, more generic, html-file publisher.
@hugo-vrijswijk @richardwerkman @simondel @Mobrockers What are your thoughts?
The text was updated successfully, but these errors were encountered: