rohanpadhye / JQF

JQF + Zest: Coverage-guided semantic fuzzing for Java.
BSD 2-Clause "Simplified" License
651 stars 110 forks source link

Split API and engine #80

Open vlsi opened 4 years ago

vlsi commented 4 years ago

Test code does not need classes like ZestCli on the classpath, so it would be nice to have API-only dependency.

rohanpadhye commented 4 years ago

That's a great point!

However, I am under the impression that other testing frameworks such as JUnit and Junit-QuickCheck also bundle the engines with the API in the test dependencies. I believe the reason is so that the test package is fully runnable, including with support for things like CLI. That is, {artifact}-test.jar should be runnable without the need for depending on the Maven plugin.

Given that app classes (i.e., not test) need not depend on even the JQF API, this should not affect packaging for production.

Thoughts?

vlsi commented 4 years ago

See JUnit5: https://junit.org/junit5/docs/current/user-guide/#running-tests-build-gradle-engines-configure

dependencies {
    testImplementation("org.junit.jupiter:junit-jupiter-api:5.5.2")
    testRuntimeOnly("org.junit.jupiter:junit-jupiter-engine:5.5.2")
}

In other words, the API classes are confined in org.junit.jupiter:junit-jupiter-api artifact, so I can't accidentally import engine classes. It just does not appear in the auto-complete.

Note: Maven does not distinguish compilation classpath and runtime classpath, so in Maven would put both api and runtime on the compilation classpath. However, Gradle can distinguish compilation and runtime, so it won't even put the engine artifact for the compilaiton classpath, which would make compilation faster, and it would reduce the scope of imports which is great (e.g. less items in the autocomplete in IDE).

I believe the reason is so that the test package is fully runnable

If you want to have a fully runnable zest.jar, it is fine (e.g. a separate jqf-zest-standallone.jar can be published), however, it would be too much for the people to import when writing @Fuzz tests.

rohanpadhye commented 4 years ago

That's a reasonable argument. Thanks for the info. It sounds like a good idea to have a new jqf-api module, which would be separate from jqf-fuzz, for the next release. Assuming that jqf-fuzz will depend on jqf-api, I guess people could simply declare a dependency on jqf-fuzz as right now if they wished to pull in the engine classes.

To implement this, we would need to move the classes JQF and Fuzz to the new jqf-api module.

The only issue right now would be that the JQF test runner class currently depends on FuzzStatement, which pulls the entire engine: https://github.com/rohanpadhye/jqf/blob/8dad26ee070d5e373b231c9f9a8ac5a94eff2e98/fuzz/src/main/java/edu/berkeley/cs/jqf/fuzz/JQF.java#L87

This line would have to be changed to load the FuzzStatement class dynamically.

Let me know if you have a cleaner solution in mind.

vlsi commented 4 years ago

@rohanpadhye , see https://github.com/pholser/junit-quickcheck/pull/255

vlsi commented 4 years ago

The only issue right now would be that the JQF test runner class currently depends on FuzzStatement, which pulls the entire engine:

What do you think if junit-quickcheck-related code was moved to its own artifact?

For instance:

So by engine for jqf I mean the ZestGuidance and alike classes. I other words, its implementation should not depend on JUnit.

WDYT?

rohanpadhye commented 4 years ago

This seems like maybe a good direction for the future but I don't know if such an engineering / refactoring effort is justified right now. We don't currently have that big of a user and/or developer base who is committed towards supporting frameworks other than junit-quickcheck such as jqwik. If the situation changes, I'll be happy to revisit this proposal.

rohanpadhye commented 4 years ago

Given the interest in jlink/jqwik#84, this refactoring may be relevant again.