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

PiranhaJava // Remove meaningless "assert" and "expect" from JUnits #47

Closed
pranavsb opened this issue Jun 9, 2020 · 5 comments · May be fixed by #151
Closed

PiranhaJava // Remove meaningless "assert" and "expect" from JUnits #47

pranavsb opened this issue Jun 9, 2020 · 5 comments · May be fixed by #151
Labels
enhancement New feature or request legacy-java Issues/PR related to older PiranhaJava implementation

Comments

@pranavsb
Copy link
Contributor

pranavsb commented Jun 9, 2020

Piranha simplification will need manual review and fixing of testcases, but I think it's useful to delete lines like:
expect(true).andReturn(true);
assertFalse(true);
which Piranha leaves behind in test classes after simplifying toggle expressions.

@pranavsb
Copy link
Contributor Author

What's the best way to approach this?

I got it working by trying some basic checks in
public Description matchMethod(MethodTree tree, VisitorState state) which is here

Very hacky code:

AnnotationTree tat =
            ASTHelpers.getAnnotationWithSimpleName(tree.getModifiers().getAnnotations(), "Test");
    if (tat != null) {
      // easymock - most common is andReturn - add more to list but have to be 100% sure
      if (tree.getBody().toString().contains("expect(true).andReturn") || tree.getBody().toString().contains("expect(false).andReturn")) {
        Description.Builder builder = buildDescription(tree);
        SuggestedFix.Builder fixBuilder = SuggestedFix.builder();
        List<? extends StatementTree> statements = tree.getBody().getStatements();
        for (StatementTree statement : statements) {
          if (isSimplifiedTestStatement(statement.toString())) {
            fixBuilder.delete(statement);
            builder.addFix(fixBuilder.build());
            return builder.build();
//            decrementAllSymbolUsages(tree, state, fixBuilder);
          }
        }
}

where isSimplifiedTestStatement just does more string matching.

Not sure if adding this logic to matchMethod would work, maybe we need to run piranha twice or modify the code to increase no. of passes.

@mkr-plse
Copy link
Contributor

Can you post the input code and expected output code? I think updating the methodProperties for these methods should handle this issue too.

@mkr-plse mkr-plse added enhancement New feature or request legacy-java Issues/PR related to older PiranhaJava implementation labels Jun 15, 2020
@pranavsb
Copy link
Contributor Author

Basically, delete the lines which are test related calls, where Piranha has already visited and substituted with a boolean

Original code

@Test
    public void testToggle() {
        ...
        int a = 1;
        expect(mockToggle.isToggleEnabled()).andReturn(true).anyTimes();
        int b = a + 1;
        ...
        assertFalse(mockToggle.isToggleEnabled());
    }

Piranha modified code

@Test
    public void testToggle() {
        ...
        int a = 1;
        expect(true).andReturn(true).anyTimes();
        int b = a + 1;
        ...
        assertFalse(true); // causes test to fail
    }

Expected code:

@Test
    public void testToggle() {
        ...
        int a = 1;
        int b = a + 1;
        ...
    }

@mkr-plse
Copy link
Contributor

This may be a slightly involved fix and it is better to avoid special casing. We will need to implement multi-pass analysis on the code to get this correct.

  1. In the first pass, determine the various Piranha refactorings and their locations, but do no rewrite the code.
  2. In the second pass, use the information from the previous pass along with standard Piranha refactoring to rewrite.

For the second pass to succeed in the case given above, methods expect, andReturn and anyTimes can be given as empty methodTypes as part of methodProperties.

Another option is to determine the code range that is updated by Piranha and re-run the standard Piranha analysis and restrict the refactorings to those regions.

Let us discuss more on the gitter channel before finalizing a solution.

@mkr-plse mkr-plse added this to the java-multipass-analysis milestone Jun 22, 2020
ketkarameya pushed a commit to ketkarameya/piranha that referenced this issue Dec 2, 2021
mkr-plse pushed a commit that referenced this issue Jan 3, 2022
* Adding support for handling junit and easy mock issue #47
* Introduced a feature that allows unnecessary test methods to be configured via the properties file
@ketkarameya
Copy link
Collaborator

This has been resolved in #156.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request legacy-java Issues/PR related to older PiranhaJava implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants