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
fix: source mapping for branch statements #518
Conversation
@coreyfarrell Is there anything I can do to help speed this up? ;-) |
@ReneSchumacher Does this PR fixes also #291 ? |
I'll write a unit test to check that (hopefully tonight if I can find the time). If it's not fixed, perhaps I can add a fix to this PR as well. |
@RoniSegal As far as I can see this has already been fixed in v15.0.0. I just updated the package reference and the statement you mentioned in the issue showed up as a branch. |
@ReneSchumacher sorry for the slow response I've been very busy lately. I will review this as soon as I can. |
Hi again, |
@coreyfarrell update? |
@coreyfarrell Any chance this will be approved? |
@coreyfarrell This is now open for almost half a year. :( Is there something missing or any other reason why this isn't considered? There are still many people waiting for the fix. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coreyfarrell This is now open for almost half a year. :( Is there something missing or any other reason why this isn't considered? There are still many people waiting for the fix.
istanbuljs gets some funding through TideLift but it's not nearly enough for me to devote the level of attention that istanbuljs needs. It's an unfortunate situation but I have to prioritize my time towards things that pay the bills.
@coreyfarrell Sorry to bother again, but do you still need anything from me before you can merge this in? |
@coreyfarrell Is there probably another maintainer who could look into this? |
@coreyfarrell I know you have limited time to support this project but this PR, which fixes an obvious bug, is not ten months old. Is there any chance it will be accepted? There are more and more people hitting the issue and it becomes quite frustrating so see a fix sitting there with no one looking at it. |
@coreyfarrell Any chance we can get this fix merged? It's been over a year now and I'm wondering if there aren't any other maintainers who might be able to help here. |
@coreyfarrell any update here? |
@ReneSchumacher thank you for the contribution, and sorry for the long time to land. As @coreyfarrell indicates, we haven't had a huge amount of time to contribute to this project the past couple years. I've been making an effort to pitch in a bit more, and am landing a few old PRs. As a follow up PR, does this need to be integrated with the Cobertura reporter? |
2 years, wow |
Hey @bcoe, thx for accepting my PR! Really happy to see this merged. I've tested the change locally and there wasn't any need to change the reporter. |
This PR fixes an issue with file coverage generation when working with transpiled code and source maps (e.g., TypeScript projects). The current implementation has a bug that uses incorrect location information for switch statements. This leads to a mismatch between the statement and branch maps in the transformed file coverage object.
This fixes #517