PHP: implemented the ability to run a test method when clicking the run icon in the gutter editor.#8142
Conversation
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
tmysik
left a comment
There was a problem hiding this comment.
Please, see my comments, thank you.
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
96cf166 to
2c2d7bc
Compare
|
Made the suggested corrections. Slightly changed the logic of the |
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
|
@jlahoda Could you tell us the way to implement this feature other than Java properly if you know? (Is this implementation OK?) |
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
2c2d7bc to
75bd763
Compare
|
Made the change. I hope I have done everything correctly now. |
|
I'll test it later. Thanks! |
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
| * then already with the method collection. | ||
| */ | ||
| TestMethodController.setTestMethods(handledDocument, Collections.emptyList()); | ||
| TestMethodController.setTestMethods(handledDocument, testMethods); |
There was a problem hiding this comment.
Perhaps better:
| TestMethodController.setTestMethods(handledDocument, testMethods); | |
| if (!testMethods.isEmpty()) { | |
| TestMethodController.setTestMethods(handledDocument, testMethods); | |
| } |
Right?
There was a problem hiding this comment.
Implementation
There was a problem hiding this comment.
Pinged @jlahoda, maybe he will have time to look into it and verify it.
There was a problem hiding this comment.
If I understand correctly, he has a different account now. @lahodaj
There was a problem hiding this comment.
Sorry for the belated reply, I've attempted to fix here:
#8201
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Show resolved
Hide resolved
tmysik
left a comment
There was a problem hiding this comment.
Looks nearly OK to me. Please, address all the unresolved comment so we can merge it. Thank you for your work.
|
@troizet I did another round of reviews, please, look at the comments. Once they are resolved, I am Ok with the merge. Let's wait also for @junichi11. Thank you for your work! 👍 |
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Outdated
Show resolved
Hide resolved
|
Please also attach screenshots(before & after). |
75bd763 to
6df55e1
Compare
|
Made the change. Also rebased to the latest master. |
tmysik
left a comment
There was a problem hiding this comment.
Looks good to me. Please, wait also for approval from @junichi11.
Thank you for your work!
junichi11
left a comment
There was a problem hiding this comment.
Looks good to me. Thank you for your work!
|
BTW, please avoid rebasing until the review is done because it's hard to see the diff (https://github.com/apache/netbeans/compare/75bd763c66853242a7dcbb5a26c4877bcd1dc87d..6df55e14fd086cdef16f870b1e7696e5616fa733) |
|
@junichi11 @tmysik Thanks so much for the review! |
php/php.project/src/org/netbeans/modules/php/project/ComputeTestMethodAnnotations.java
Show resolved
Hide resolved
…un icon in the gutter editor.
6df55e1 to
50f1c09
Compare
|
Let's merge this after CI passes because the feature freeze is on January 24th. |

Before:
After:
Algorithm Description:
Each time a file is opened or edited, it checks for test methods in the file and uses
TestMethodController.setTestMethodsto create annotations that use theRunDebugTestGutterAction.javaaction to execute the test method.Peculiarities of implementation:
The implementation was done by analogy with the implementation of annotations for java (ComputeTestMethodsImpl.java).
Due to the fact that I have not found an analog of
EditorAwareJavaSourceTaskFactoryfor php, I decided to make a binding to the event of getting the focus of the editor tab and changes in the edited document.The php project opening hook seemed to me to be a less appropriate place to register a listener. Since events are handled for each editor tab regardless of the project and file type, I made the
ComputeTestMethodAnnotationsclass a singleton to avoid having to register the listener multiple times if multiple php projects are open.TestMethodController.setTestMethodsimplements all the work of creating an annotation to run a test method.Apparently, this method should update the list of annotations at each call of this method, when the passed collection of methods changes. Вut I didn't manage to achieve correct work in this case. While debugging the ComputeTestMethodsImpl.java class for java, I noticed that the method is called twice: first for an empty collection,
then for a collection with new computed methods.
So I made the method call first with the empty collection, to clear the annotation list, then already with the method collection.
The algorithm of searching for test methods was made by analogy as in the implementation of action for launching a test method for execution from the context menu of the editor.