tntim96 / JSCover

JSCover is a JavaScript Code Coverage Tool that measures line, branch and function coverage
GNU General Public License v2.0
399 stars 84 forks source link

Directly instrument code rather than files? #283

Closed tdtimothy closed 4 years ago

tdtimothy commented 4 years ago

I've downloaded this as a library with Maven and I'm trying to instrument code as a string rather than having to write that as a file, generating the instrumented file from it, and then reading from that file again. Scouring through the code, I see that the SourceProcessor is what would allow me to do that, but it's not open to the user end. Would I be able to do this another way, access SourceProcessor through another method, or whether this is planned to be supported at any point?

tntim96 commented 4 years ago

THat shouldn't be too hard to do. Probably in a similar way to https://github.com/tntim96/JSCover/pull/100. Did you want to try to implement the change yourself?

tdtimothy commented 4 years ago

Yeah, should be pretty simple thing. I'm thinking of just overloading the methods in the InstrumenterService to have a String parameter in place of the File parameter similar to how instrumentJSForProxyServer is set up. In addition, was thinking of adding a String method equivalent for instrumentJSForFileSystem since the other methods return String already and it'd be a lot more convenient for me. Would there be any issues if I took this approach?

Edit: That said, could I ask why the SourceProcessor class isn't a public class? Is it because there's use of a lot of google libraries?

tntim96 commented 4 years ago

So just confirming you need this via the API, and not via the command line?

could I ask why the SourceProcessor class isn't a public class?

I guess I've tried to minimize the exposed API to what's needed by JSCover-maven-plugin, which doesn't include InstrumenterService, however that does look like a reasonable class to add the functionality...

tdtimothy commented 4 years ago

Yeah, planning to use it purely through the API, though I don't think adding capability to process it through the command line would be too difficult either if you'd like me to do it.

tntim96 commented 4 years ago

Does the above fix do what you need?

I've released it in the 2.0.10 SNAMPSHOT if you'd like to test.

<repositories>
    <repository>
        <id>sonatype-nexus-snapshots</id>
        <name>Sonatype Nexus Snapshots</name>
        <url>https://oss.sonatype.org/content/repositories/snapshots</url>
        <snapshots>
            <enabled>true</enabled>
        </snapshots>
    </repository>
</repositories>
tdtimothy commented 4 years ago

Yeah, it looks good, thank you! I'll have to do some testing, but it should be fine. Probably.

tntim96 commented 4 years ago

Let me know when all is OK and I'll release.

tdtimothy commented 4 years ago

Hmm this might be complicated than I expected. I had expected most of the instrumentation to be done directly to the source code given the method basically returned a string/a string written into a file, but there's a lot more URI stuff than expected. There might need to be a major rework to make what I want work. Might be better off looking for alternatives.

Would there be a way to instrument code directly and have the URI just be replaced by a name/string indicating it's origins? Do you have any recommendations for that or is this just unfeasible at the time?

Might have misread a couple things, can probably make this work. Sorry about the trouble. Probably wrap up testing by end of week at latest.

Looks like it instruments just fine. I'll bring up if I encounter further issues, but thank you for the help.

tntim96 commented 4 years ago

Great. There's no rush.