Comply with html reporter format

[hugo-vrijswijk]

Closes #29

What it does

Supports the new HTML reporter schema. (stryker-mutator/mutation-testing-elements#1)

How it works

Adds some new case classes that when converted to JSON, comply with the schema. Root case class has a toJson method that returns a io.circe JSON object. Case classes do not have html in their names, as the schema is not html-report specific (though that is the only implementation for now).

Also adds a mapper that converts the MutantRunResult to the case classes for the HTML report.

This PR is to a new branch, feat-html-reporter instead of the already existing branch HTML-reporter as I have made a lot of changes, merged master, etc, which would create a really, really ugly diff.

[Wmaarts]

I have a couple of thoughts about this trait.

1. There's one test that covers all the methods, shouldn't we split this up more? That way if a test fails it's easier to identify where the bug is, but I'm not sure how important that is.
2. If we're not going to do nr 1, all of these methods can be marked private except toReport, if I'm correct.
3. The method names slightly throw me off. i.e. toLocation makes more sense as an extension method. Now for example you're calling toMutantStatus, but I'm thinking: "you're going to MutantStatus... from where exactly?" if that makes sense. To me something like mapRunResultToMutantStatus makes more sense.

[hugo-vrijswijk]

1. There's one test that covers all the methods, shouldn't we split this up more? That way if a test fails it's easier to identify where the bug is, but I'm not sure how important that is.

I agree. The single test is a bit massive. The thing is though is that the 'top' map function calls the other functions anyway. So then having tests for the 'smaller' functions is a bit redundant

3. The method names slightly throw me off. i.e. `toLocation` makes more sense as an extension method. Now for example you're calling `toMutantStatus`, but I'm thinking: "you're going to MutantStatus... from where exactly?" if that makes sense. To me something like `mapRunResultToMutantStatus` makes more sense.

Hmm, I feel like the "from where" is already apparent from it being given as a parameter (as well as the signature). Having names like mapRunResultToMutantStatus will get pretty big and unreadable. I agree the names could be better though

[nicojs]

Are you guys checking in the build result of mutation-testing-elements? Please don't do that! Suggestion: add an SBT plugin to download the file in the build 🙈

[hugo-vrijswijk]

Are you guys checking in the build result of mutation-testing-elements? Please don't do that! Suggestion: add an SBT plugin to download the file in the build 🙈

Cool idea! I agree it's a bit ugly to have the generated js in source control, but I'm not sure what the alternative should be. Ideally, the file would be downloaded at the same as the other dependencies. But to do that we'd have to upload it to maven central or something.

If we were to download it with an SBT plugin we still need to put the file somewhere. Putting it in resources means we'd have to add it to gitignore (not a terrible idea), putting it in target/... opens up a whole new can of worms.

Alternatively, maybe something like webjars could be useful for us.

[nicojs]

If we were to download it with an SBT plugin we still need to put the file somewhere. Putting it in resources means we'd have to add it to gitignore (not a terrible idea)

Sounds like a good first step. As a performance improvement: don't download it if the file is already there and remove it on sbt clean (or whatever the alternative is to mvn clean).

Ideally, the file would be downloaded at the same as the other dependencies. But to do that we'd have to upload it to maven central or something.

Sounds good to me. You could also create a package.json file and simply run npm install. You then manage the version from there. The file can be found in node_modules/mutation-testing-elements/dist/mutation-test-elements.js.

[hugo-vrijswijk]

You could also create a package.json file and simply run npm install

Not sure if I like having to install npm as a requirement for our build. For now, I've made a PR to add it as a dependency

[nicojs]

Not sure if I like having to install npm as a requirement for our build

Yeah, I agree that multiple development stacks as a build requirement isn't the most elegant solution. Glad to see there is a way around it with webjars.

[hugo-vrijswijk]

@Wmaarts I've made some changes. Please have a look and see if you agree

[Wmaarts]

Looks good! I only have questions tbh.

[Wmaarts]

Any reason this is private instead of private[this]? (I'm actually not sure about the difference. 🙈 )

[hugo-vrijswijk]

There's no real reason other than consistency. private means objects of other types can not access it. private[this] means objects of the same type also cannot access it. So it's slightly tighter encapsulation. (You can also have protected[com.packagename] for example). It's explained better here

[Wmaarts]

I don't think there's consistency on this throughout stryker4s though.

[hugo-vrijswijk]

True. But now there is within the class

[Wmaarts]

I guess we'll save this discussion for later then. x)

outdated