Throw specific exceptions based on the method under testFeature/zfj/add exception on actual #29

zhangfj88 · 2024-03-25T07:18:08Z

Throw specific exceptions based on the method under test

Add eThrow specific exceptions based on the method under testxception on actual

yaronyam

thanks for contributing this feature. I fear that most users might prefer the simplicity of specifying a general Exception type. would be good if a UI settings can be added to control this feature - i.e. "declare specific test method thrown exception types" - with checkbox defaulted to false

yaronyam · 2024-03-25T17:03:16Z

src/main/java/com/weirddev/testme/intellij/builder/MethodFactory.java

@@ -22,6 +22,7 @@
 import com.weirddev.testme.intellij.utils.JavaPsiTreeUtils;
 import com.weirddev.testme.intellij.utils.PropertyUtils;
 import com.weirddev.testme.intellij.utils.TypeUtils;
+import org.apache.commons.lang.StringUtils;


please avoid dependencies on apache commons. it's a transitive dependency added by internal implementation of Jetbrains and may be removed. furthermore, I prefer to maintain small installation size and minimize external dependencies

Thank you for your suggestion.I will try to make modifications according to your advice.

yaronyam · 2024-03-25T17:22:23Z

src/main/java/com/weirddev/testme/intellij/builder/MethodFactory.java

@@ -56,7 +57,8 @@ public static Method createMethod(PsiMethod psiMethod, PsiClass srcClass, int ma
        Optional<PsiSubstitutor> methodSubstitutor = findMethodSubstitutor(psiMethod, srcClass, ownerClassPsiType);
        Type returnType = resolveReturnType(psiMethod, maxRecursionDepth, typeDictionary, methodSubstitutor);
        List<Param> methodParams = extractMethodParams(psiMethod, isPrimaryConstructor, maxRecursionDepth, typeDictionary, methodSubstitutor);
-        return new Method(methodId, methodName, returnType,   ownerClassCanonicalType, methodParams, isPrivate, isProtected, isDefault, isPublic, isAbstract, isNative,
+        String methodExceptionTypes = extractMethodExceptionTypes(psiMethod);


please rename all references to throwsExceptions

yaronyam · 2024-03-25T17:26:41Z

src/main/java/com/weirddev/testme/intellij/builder/MethodFactory.java

+        return methodExceptionTypes;
+    }
+
+    private static String getExceptionTypeName(String exceptionTypeName) {


fully qualified name must be used in generated code. otherwise it may not compile due to missing import statement. a similar comment with example was already posted on previous PR - https://github.com/wrdv/testme-idea/pull/27/files#r1532873546

yaronyam · 2024-03-25T17:39:09Z

src/main/java/com/weirddev/testme/intellij/builder/MethodFactory.java

+
+    //analyze the exception types of the method
+    private static String extractMethodExceptionTypes(PsiMethod psiMethod) {
+        String methodExceptionTypes = "";


some of the following actions on psi tree could be expensive in terms of performance. better check first if psiMethod does not below to tested class - return null.
BTW, default to null rather than empty string

yaronyam · 2024-03-25T18:03:13Z

src/main/java/com/weirddev/testme/intellij/template/context/Method.java

+     * true - if method has Exception
+     */
+    public boolean hasException(){
+        return StringUtils.isNotEmpty(methodExceptionTypes);


think we can do without this method. better simplify and init field to null if resolved as empty string

yaronyam · 2024-03-25T18:04:42Z

src/main/resources/fileTemplates/testMeTests/JUnit4 & Mockito.java.ft

@@ -29,7 +29,7 @@ public class ${CLASS_NAME} {
 #if($TestSubjectUtils.shouldBeTested($method))

    @Test
-    public void #renderTestMethodName($method.name)() throws Exception {
+    public void #renderTestMethodName($method.name)() #if($method.hasException()) throws $method.methodExceptionTypes #end{


remove redundant spaces. before #if and #end. comment relevant for other templates as well

…pes"

# Conflicts: # src/main/java/com/weirddev/testme/intellij/configuration/TestMeConfig.java # src/main/java/com/weirddev/testme/intellij/template/FileTemplateConfig.java # src/main/java/com/weirddev/testme/intellij/ui/settings/TestMeSettingsForm.form # src/main/java/com/weirddev/testme/intellij/ui/settings/TestMeSettingsForm.java

Throw specific exceptions based on the method and configuration

yaronyam

Great PR 💪 . Would be even greater if at least 1 integration test would be added. review TestMeGeneratorJunit4Test#testOverrideAbstractIgnoreInherited for example of a test case effected by configuration setting. if you're short on time - let me know I'll add one later. Thank You

yaronyam · 2024-04-03T19:04:58Z

src/main/java/com/weirddev/testme/intellij/builder/MethodFactory.java

+            PsiReferenceList throwsList = psiMethod.getThrowsList();
+            PsiClassType[] referencedTypes = throwsList.getReferencedTypes();
+
+            for (PsiClassType type : referencedTypes) {


can be simplified. please replace with:

String throwsExceptions = Arrays.stream(referencedTypes) .map(PsiClassType::resolve) .filter(Objects::nonNull) .map(PsiClass::getQualifiedName) .filter(Objects::nonNull) .collect(Collectors.joining(",")); return throwsException.isEmpty() ? "Exception" : throwsException;

thanks，I will Modify as per your advice and add integration test💪

yaronyam · 2024-04-03T19:11:32Z

src/main/java/com/weirddev/testme/intellij/template/context/TestMeTemplateParams.java

@@ -89,4 +89,6 @@ public interface TestMeTemplateParams {
     * Language of class under test: Groovy, Java, Scala
     */
    String TESTED_CLASS_LANGUAGE = "TESTED_CLASS_LANGUAGE";
+
+    String THROW_SPECIFIC_EXCEPTION_TYPES = "throwSpecificExceptionTypes";


seems unused. please remove

yaronyam · 2024-04-03T19:42:08Z

src/main/java/com/weirddev/testme/intellij/ui/settings/TestMeSettingsForm.form

+              <grid row="3" column="0" row-span="1" col-span="1" vsize-policy="0" hsize-policy="3" anchor="8" fill="0" indent="0" use-parent-layout="false"/>
+            </constraints>
+            <properties>
+              <text value="declare specific test method thrown exception types"/>


please Capitalize: "Declare ...

…integration test

Throw specific exceptions based on the method and configuration--add integration test

…integration test

Throw specific exceptions based on the method and configuration--add integration test

yaronyam · 2024-04-09T13:50:38Z

src/main/java/com/weirddev/testme/intellij/builder/MethodFactory.java

+                    .filter(Objects::nonNull)
+                    .collect(Collectors.joining(","));
+        }
+        return throwsExceptions == null ? "Exception" : throwsExceptions;


I think in case non found - throwsException would be empty string rather then null, if so please replace with
return throwsException.isEmpty() ? "Exception" : throwsException;
would also be good to change/add integration test to throw a more specific exception (i.e. java.io.IOException) rather then Exception in order to verify mapping logic.

ok，Let me modify the code then submit it

in case non found - throwsException would be empty string rather then null --YES

in case "Declare specific test method thrown exception types " is not checked --throwsException is null

in cased "Declare specific test method thrown exception types " is checked and The testing method does not contain any exceptions--- then test method does not throw any exceptions.
I prefer to case 3，
so ,you prefer to throwing Exception in case 3?

I see what you mean...thanks for elaborating
Ok, lets keep this logic as you suggest. Would like to refine the UI text to better express this into:
"Declare exceptions thrown by tested class" - so it would suggest that only if tested class actually declares exceptions - so will the test method.

Furthermore please do a small refactor... I find that mutating throwsExceptions variable and using it to transitively reflect throwSpecificExceptionTypes state - a bit confusing.

please use this alternative:

private static String extractMethodExceptionTypes(PsiMethod psiMethod ,Boolean throwSpecificExceptionTypes,boolean testable) { if(!testable){ return null; } if(throwSpecificExceptionTypes){ PsiReferenceList throwsList = psiMethod.getThrowsList(); PsiClassType[] referencedTypes = throwsList.getReferencedTypes(); String throwsExceptions = Arrays.stream(referencedTypes) .map(PsiClassType::resolve) .filter(Objects::nonNull) .map(PsiClass::getQualifiedName) .filter(Objects::nonNull) .collect(Collectors.joining(",")); return throwsExceptions.isEmpty() ? null : throwsExceptions; } else { return "Exception"; } }

Thank you for optimizing the code. I am modifying the code and then submit.

…integration test

Throw specific exceptions based on the method and configuration--add integration test

yaronyam · 2024-04-12T16:39:23Z

Released in TestMe plugin version 6.4

zhangfj88 and others added 6 commits March 20, 2024 16:59

Throw specific exceptions based on the method under test

8d068d4

Throw specific exceptions based on the method under test

7cfcfc6

Merge pull request #3 from zhangfj88/add-exception-on-actual

038ba02

Add eThrow specific exceptions based on the method under testxception on actual

Throw specific exceptions based on the method under test

e17994d

Throw specific exceptions based on the method under test

8cba2c5

Throw specific exceptions based on the method under test

3cb7ef0

yaronyam reviewed Mar 25, 2024

View reviewed changes

zhangfj88 and others added 10 commits March 26, 2024 11:43

Add a UI setting as "declare specific test method thrown exception ty…

1671e0b

…pes"

Throw specific exceptions based on the method and configuration

86a47ab

Throw specific exceptions based on the method and configuration

0579e4f

Throw specific exceptions based on the method and configuration

ce60478

Throw specific exceptions based on the method and configuration

3385ccf

Merge pull request #7 from zhangfj88/add-exception-on-actual

3dc4ae1

Throw specific exceptions based on the method and configuration

Throw specific exceptions based on the method and configuration

d1a7e97

Throw specific exceptions based on the method and configuration

f0e8d73

Merge pull request #8 from zhangfj88/add-exception-on-actual

24f4d08

Throw specific exceptions based on the method and configuration

yaronyam reviewed Apr 3, 2024

View reviewed changes

zhangfj88 and others added 5 commits April 8, 2024 11:42

Throw specific exceptions based on the method and configuration

30e421a

Throw specific exceptions based on the method and configuration--add …

618b035

…integration test

Merge pull request #9 from zhangfj88/add-exception-on-actual

69343c7

Throw specific exceptions based on the method and configuration--add integration test

Throw specific exceptions based on the method and configuration--add …

43f6a16

…integration test

Merge pull request #10 from zhangfj88/add-exception-on-actual

558874e

Throw specific exceptions based on the method and configuration--add integration test

yaronyam reviewed Apr 9, 2024

View reviewed changes

zhangfj88 and others added 2 commits April 11, 2024 11:15

Throw specific exceptions based on the method and configuration--add …

22c3d16

…integration test

Merge pull request #11 from zhangfj88/add-exception-on-actual

b262555

Throw specific exceptions based on the method and configuration--add integration test

yaronyam approved these changes Apr 11, 2024

View reviewed changes

yaronyam merged commit 9702063 into wrdv:master Apr 11, 2024
5 checks passed

yaronyam mentioned this pull request Apr 12, 2024

when "Declare exceptions thrown by tested class" setting is on - test code exceptions should be taken into account #32

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Throw specific exceptions based on the method under testFeature/zfj/add exception on actual #29

Throw specific exceptions based on the method under testFeature/zfj/add exception on actual #29

zhangfj88 commented Mar 25, 2024

yaronyam left a comment

yaronyam Mar 25, 2024

zhangfj88 Mar 26, 2024

yaronyam Mar 25, 2024

yaronyam Mar 25, 2024

yaronyam Mar 25, 2024

yaronyam Mar 25, 2024

yaronyam Mar 25, 2024

yaronyam left a comment

yaronyam Apr 3, 2024

zhangfj88 Apr 7, 2024

yaronyam Apr 3, 2024

yaronyam Apr 3, 2024

yaronyam Apr 9, 2024

zhangfj88 Apr 10, 2024

zhangfj88 Apr 10, 2024

yaronyam Apr 10, 2024

zhangfj88 Apr 11, 2024

yaronyam commented Apr 12, 2024

Throw specific exceptions based on the method under testFeature/zfj/add exception on actual #29

Throw specific exceptions based on the method under testFeature/zfj/add exception on actual #29

Conversation

zhangfj88 commented Mar 25, 2024

yaronyam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaronyam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yaronyam commented Apr 12, 2024