Stryker test failures on GitHub

[brodybits]

Quoting @karfau from #192 (comment):

the new tests added as part of the security fix break the stryker setup, since they rely on line numbers in snapshots, and stryker mutates the files causing line numbers to change

---

P.S. I am wondering if the most recent Stryker update would suffer from this issue or not.

[brodybits]

I have pinned this issue in first place and think we need to resolve this before merging any more PRs.

I have volunteered to take assignment for now. Unfortunately I cannot promise when I will get a chance to work on this due to some other priorities. I would not mind if any other maintainer wants to take this one over.

[karfau]

Thank you, I agree that we should resolve this.

When i find time I will also start looking into it and report here, when I do that.

I need to look at the code again and remember/document why I added the line numbers to the assertion, right now I don't remember... :sad:

But assuming that we need those assertions, I can think of two approaches to solve it differently:

A) Add unique error codes to each error that is tested and add it to the snapshot instead of the line number. Problems:

• We change the production code for our tests, but maybe its even a good idea to have those codes, in case someone reports a bug?
• We would need to make sure they are unique, which could be tested by reading the files.

B) To not change the production code, we could read the production files in a beforeAll and create a map from line numbers to a numeric index of the errors that can be thrown. We then use the index instead of the line number when writing/comparing to the snapshot. A bit more complexity in the tests, but I would say that's worth it.

[brodybits]

Thanks for the response. It would be awesome if you get a chance to work on this. I hope we (you?) can find a simple solution for this.

I think it would be ideal if we can get this reported and fixed on the Stryker side, but maybe just easier to fix on our side :)

[karfau]

I have pinned this issue in first place and think we need to resolve this before merging any more PRs.

Should this issue block updating the changelog and releasing 0.6.0?

[brodybits]

Yes, and I think we should fix the build before merging anything else into master, unless there is something super urgent.

[karfau]

I think it would be ideal if we can get this reported and fixed on the Stryker side, but maybe just easier to fix on our side :)

I don't think they would consider it an issue on their side since all the docs say that the mutations change the code and with version 4 that puts all mutations into the files at the same time, I can not imagine that they could do that without any change in line numbers.

[karfau]

Started looking into it and already made some progress.

[karfau]

@brodybits Everything we added to the 0.6.0 milestone is done now. Can you take care of updating the changelog and publishing the next release? (I think the devDependency updates that are pending shouldn't stop us from releasing.)

Thank you

[brodybits]

The screenshot in PR #201 shows a score of 71%, while the dashboard shows just over 65%. I wonder if the dashboard is not updating for some reason?

I think it would be ideal if we could update Stryker to 4.5.1, as proposed in PR #192. I would be happy to test that update but have very little time this week.

I would probably need 1-2 weeks to find time to make the 0.6.0 release. @karfau I think you should be able to publish this one. I would be happy to do a quick review if you like.