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

JUnitAssertRefactoring - different opinion for "fail" refactorings #277

Open
cal101 opened this issue Jul 22, 2017 · 2 comments
Open

JUnitAssertRefactoring - different opinion for "fail" refactorings #277

cal101 opened this issue Jul 22, 2017 · 2 comments

Comments

@cal101
Copy link
Collaborator

cal101 commented Jul 22, 2017

Hi @JnRouvignac @Fabrice-TIERCELIN,

using the experimental command line interface of autorefactor I am trying refactorings on
some bigger source sets.

I understand that autorefactor is an opinionated sets of rules and I am agreeing with most of those opinions. But in case of JUnitAssertRefactoring I disagree with (some of) the refactorings of "fail".

For example look at the following diff:

-            if (null != results) {
-                fail(m + "query failed XPath: " + results +
-                        "\n xml response was: " + response +
-                        "\n request was: " + req.getParamString());
-            }
+            assertNull(m + "query failed XPath: " + results +
+                               "\n xml response was: " + response +
+                               "\n request was: " + req.getParamString(), results);

"result" and or "response" may be big strings or objects having an expensive toString or producing big strings.
So this code is written this way to avoid those costs.
Something along the logging idiom

if (log.isDebugEnabled()) {
    log.debug(<expensive-to-string>);
}

Would you consider some kind of configuration in cases like this?
If you can tell me an example of how to configure a refactoring I may be able to develop something for this case.

@Fabrice-TIERCELIN
Copy link
Collaborator

You're right. Currently, you can disable one refactoring going to Window -> Preferences -> Java -> Code style -> AutoRefactor -> Rules by default. In the future, we should split the JUnit rule to allow to enable/disable only this part of the rule. We planned to do such split for the next version (not the next release) but we don't know when we will release.

@cal101
Copy link
Collaborator Author

cal101 commented Jul 23, 2017

Do you have any prototype work or ideas how to do it?
I think about giving names to the different parts and being able to enable or disable parts or configure them.
Something like

--param JUnitAssertRefactoring.ifToFail=false

and the rule providing Parameter objects including descriptions.
Something that allows ui usage and command line usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants