PHP: implemented the ability to run a test method when clicking the run icon in the gutter editor.

[troizet]

Before:

After:

Algorithm Description:

Each time a file is opened or edited, it checks for test methods in the file and uses TestMethodController.setTestMethods to create annotations that use the RunDebugTestGutterAction.java action 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 EditorAwareJavaSourceTaskFactory for 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 ComputeTestMethodAnnotations class a singleton to avoid having to register the listener multiple times if multiple php projects are open.

• TestMethodController.setTestMethods implements 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.

[tmysik]

Please, see my comments, thank you.

[junichi11]

Thank you for your work.

[troizet]

Made the suggested corrections. Slightly changed the logic of the getTestMethods method.

[junichi11]

@jlahoda Could you tell us the way to implement this feature other than Java properly if you know? (Is this implementation OK?)

[troizet]

Made the change. I hope I have done everything correctly now.

[junichi11]

I'll test it later. Thanks!

[tmysik]

Perhaps better:

Suggested change

	TestMethodController.setTestMethods(handledDocument, testMethods);
	if (!testMethods.isEmpty()) {
	TestMethodController.setTestMethods(handledDocument, testMethods);
	}

Right?

[junichi11]

I've verified that we cannot get the expected behavior if we don't set an empty list...
(A popup list is not shown after we change a method's name.)

[tmysik]

Interesting.

[junichi11]

Implementation

netbeans/ide/gsf.testrunner.ui/src/org/netbeans/modules/gsf/testrunner/ui/api/TestMethodController.java

Lines 48 to 84 in 5df292f

	private static void setTestMethodsImpl(StyledDocument doc, List<TestMethod> methods) {
	doc.putProperty(TestMethodAnnotation.DOCUMENT_METHODS_KEY, methods);
	Map<TestMethod, TestMethodAnnotation> annotations = (Map<TestMethod, TestMethodAnnotation>) doc.getProperty(TestMethodAnnotation.DOCUMENT_ANNOTATIONS_KEY);
	if (annotations == null) {
	annotations = new HashMap<>();
	doc.putProperty(TestMethodAnnotation.DOCUMENT_ANNOTATIONS_KEY, annotations);
	}
	Map<Integer, TestMethod> annotationLines = (Map<Integer, TestMethod>) doc.getProperty(TestMethodAnnotation.DOCUMENT_ANNOTATION_LINES_KEY);
	if (annotationLines == null) {
	annotationLines = new HashMap<>();
	doc.putProperty(TestMethodAnnotation.DOCUMENT_ANNOTATION_LINES_KEY, annotationLines);
	}
	Set<TestMethod> added = new HashSet<>(methods);
	Map<TestMethod, TestMethodAnnotation> removed = new HashMap<>(annotations);
	removed.keySet().removeAll(added);
	added.removeAll(annotations.keySet());
	for (TestMethod method : added) {
	TestMethodAnnotation a = new TestMethodAnnotation(method);
	NbDocument.addAnnotation(doc, method.preferred, 0, a);
	annotations.put(method, a);
	int line = NbDocument.findLineNumber(doc, method.preferred.getOffset());
	annotationLines.put(line, method);
	}
	for (Entry<TestMethod, TestMethodAnnotation> e : removed.entrySet()) {
	NbDocument.removeAnnotation(doc, e.getValue());
	annotations.remove(e.getKey());
	int line = NbDocument.findLineNumber(doc, e.getKey().preferred.getOffset());
	annotationLines.remove(line);
	}
	}

[tmysik]

Pinged @jlahoda, maybe he will have time to look into it and verify it.

[tmysik]

Looks nearly OK to me. Please, address all the unresolved comment so we can merge it. Thank you for your work.

[tmysik]

@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! 👍

[junichi11]

Please also attach screenshots(before & after).

[troizet]

Made the change. Also rebased to the latest master.

[tmysik]

Looks good to me. Please, wait also for approval from @junichi11.

Thank you for your work!

[junichi11]

Looks good to me. Thank you for your work!

[junichi11]

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)

[troizet]

@junichi11 @tmysik Thanks so much for the review!

[junichi11]

Let's merge this after CI passes because the feature freeze is on January 24th.