Skip to content
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

Reduce code duplication in processor test suite with parameterized tests #141

Closed
slarse opened this issue Oct 13, 2020 · 3 comments
Closed
Labels
test Something related to the test suite or CI

Comments

@slarse
Copy link
Collaborator

slarse commented Oct 13, 2020

I've been poking around the test suite for the processors (i.e. src/test/java/sorald/processor), and I've found that there is a whole lot of code duplication. Most of the test classes are borderline identical, and generally follow this pattern:

public class SomeCheckProcessorTest {
    @Test
    public void test() throws Exception {
        String fileName = "SomeFile.java";
        String pathToBuggyFile = Constants.PATH_TO_RESOURCES_FOLDER + fileName;
        String pathToRepairedFile = Constants.SORALD_WORKSPACE + "/" + Constants.SPOONED +"/" + fileName;

        JavaCheckVerifier.verify(pathToBuggyFile, new SomeCheck());
        Main.main(new String[]{
                Constants.ARG_SYMBOL + Constants.ARG_ORIGINAL_FILES_PATH,pathToBuggyFile,
                Constants.ARG_SYMBOL + Constants.ARG_RULE_KEYS,"<KEY>",
                Constants.ARG_SYMBOL + Constants.ARG_WORKSPACE,Constants.SORALD_WORKSPACE});
        TestHelper.removeComplianceComments(pathToRepairedFile);
        JavaCheckVerifier.verifyNoIssue(pathToRepairedFile, new ArrayHashCodeAndToStringCheck());
    }
}

There is some minor variation in the arguments to Main.main and variable names, but most classes are otherwise semantically identical. If Sorald is to scale with more rules and more tests, I think this must be revamped.

As a solution, I propose that we write a single parameterized test, and keep a directory with "broken" Java files (such as the current src/test/resources directory). Then, each file can be automatically turned into a separate test case. If we for a moment ignore the problem with the variation in command line arguments, a test case file could be named as SomethingInformative_<SONAR_CUBE_KEY>.java, and be used to generate a test case for the rule with key <SONAR_CUBE_KEY> (e.g. EqualsOnAtomicClass_2204.java to test rule 2204).

I have yet to figure out the best solution for the variation in arguments to Main.main, but I'm sure a reasonable solution can be found through some trial and error.

As an example, I've done something similar to this in Spork, an AST-based merge tool. There, I have one directory per test case containing the three input files for the merge, as well as the expected output file as an oracle. Then, source providers that look like this generate input for a handful of test cases that look like this. This gives me hundreds of test cases with very few lines of code, and adding a new test case only requires the addition of new input, there's no additional logic.

I could get to work on this right away. Any thoughts?

@fermadeiral
Copy link
Collaborator

I could get to work on this right away.

The solution you proposed seems good to me. I encourage you to proceed.

@slarse
Copy link
Collaborator Author

slarse commented Oct 13, 2020

Cool! If I haven't overlooked something critical, I should at least have a draft PR of this sometime tomorrow.

@slarse
Copy link
Collaborator Author

slarse commented Oct 22, 2020

This was fixed with #144

@slarse slarse closed this as completed Oct 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Something related to the test suite or CI
Development

No branches or pull requests

2 participants