Show progress when searching test methods in JUnit run configuration #653

[fedejeanne]

What it does

Move the logic to search for test methods from JUnitLaunchConfigurationTab to TestSearchEngine::findTestMethods. Extract the whole logic for caching the results into TestMethodsCache (new class) and do the whole searching and caching more efficiently i.e. only when necessary and showing the progress with a monitor by using ModalContext::run

Fixes #683
Contributes to #653

This PR shows a progress indicator when searching for test methods in a class (which may happen when one opens a JUnit run configuration). The progress indicator looks like this when you first open the Run configurations and a JUnit configuration is selected:

... and like this when you select or edit an existing JUnit run configuration and force a new search for test methods (the progress indicator appears at the bottom of the Run configurations dialog).

How to test

The main goal is to open some Run configurations for JUnit tests and move between them, letting them search the necessary test methods.

All of the following must hold true while testing:

• If the UI is blocked (which is normal) then some progress report should appear, either in a separate dialog (when opening the Run configurations for the first time) or in a progress bar at the bottom of the Run configurations dialog.
• Previous results from the same run configuration (project + class + JUnit runner) should be cached e.g. if you already found all test methods for class A in project B and for JUnit5 then changing to class A2 might trigger a search (if A2 exists) but setting the class back to A should not trigger another search.
• Selecting another configuration in the dialog (on the left-side panel) empties the cache.
• If you cancel a search then the Test method (text field) and its Search button should be deactivated since the methods of the class are not known.
  
• It follows that if you cancel a search then the configuration is invalid i.e. the Run button should be deactivated and an error should be displayed in the notification area of the dialog (upper area).

A small hack to help testing

If you need more time to see the progress dialog/bar then you can modify TestSearchEngine::collectMethodNames like this:

	private static void collectMethodNames(IType type, IJavaProject javaProject, String testKindId, Set<String> methodNames, IProgressMonitor monitor) throws JavaModelException {
		if (type == null) {
			return;
		}

		SubMonitor subMonitor= SubMonitor.convert(monitor, 3 + 3); // Add +3

		// Add this idle wait of 3 seconds and advance the progress in the subMonitor
		for (int i= 0; i < 3; i++) {
			try {
				Thread.sleep(1_000);
				subMonitor.split(1);
			} catch (InterruptedException e) {
				// ignore
			}
		}

		// leave the rest unchanged
                ...
	}

How did I test?

These are all the stuff I did and tried when testing:

• I created a hierarchy of 2 test classes so that I may run tests in the super-class from the sub-class
• I added test methods both in the super-class and in the sub-class
• I created Run configurations for the test methods and also for the complete classes (i.e. left the Test method empty in the configuration dialog).
• I created a completely empty run configuration which will never be able to run but it helps to empty the cache without closing the Run configurations dialog (selecting it empties the cache of the previous JUnit run configuration).
• I edited the existing JUnit configuration in every possible way I could imagine, entering both valid and invalid values for the project, test class and test method and also switching between JUnit3, 4 and 5 in the drop-down menu ("Test runner")
• I also used the Browse... and Search buttons to change the project and the class in the run configuration
• I sometimes canceled the search for test methods
• I changed the project / test class / test runner back and forth several times (without selecting another Run configuration from the left-side panel) and purposely triggered new searches for test methods.
• I completely emptied the cache by selecting another (an empty) run configuration in the left-side panel --> no searches should occur if the configuration is empty

Author checklist

• ✔ I have thoroughly tested my changes
• ✔ The change is following the coding conventions
• ✔ I have signed the Eclipse Contributor Agreement (ECA)

[fedejeanne]

Random test failure of SaveParticipantTest.testFormatChangeBug561164 already documented in #79

[fedejeanne]

Hello all,
there's been no activity on this one in the past few weeks.
Please let me know if there's anything I can do to help reviewing this PR (more details/screenshots/comments).

I just rebased and solved the conflicts to ease the review.

[jjohnstn]

@fedejeanne Will take a look

[jjohnstn]

Tried it out. Works well, but have the following comment.

I used your delay code to be able to cancel a search. When a search is cancelled, it would be helpful if the error message states that it cannot find method after cancelled search. After cancelling, I believe the search button for method should remain active and if pressed, should restart the search. Otherwise, after cancelling a search a user has no options other than to close without saving. I had a case where it was running all tests and after cancelling it left the Run button active because it did not have to do the verification which is fine.

[fedejeanne]

Thank you for reviewing and testing!

I added a hint that says the user canceled the operation in order to let the user know why can't the method be found.
Additionally, the Search... button now remains active even after cancellation and triggers the search again if necessary.

[jjohnstn]

Hi @fedejeanne Thanks for the changes. Could you take a look at the test failures? Monday is cut-off for M3 so if we can't get this in by then, it will have to wait until 4.30 M1.

[jjohnstn]

There was a fix recently in jdt.ui for Junit 4/5 failures. Please rebase on master (don't merge) and do a force push if needed.

[jjohnstn]

I made the rebase for you. I had a similar problem with a PR and rebasing fixed the test failures.

[jjohnstn]

Thanks @fedejeanne I made a couple tweaks (no logging interrupt exception and fixing error message to not spit out null when the testmethods field not filled in). Merged for M3.

[fedejeanne]

Thank you very much for the review, changes and merging @jjohnstn ! And I'm glad to see that it's a noteworthy change!

[jjohnstn]

@fedejeanne With regards to noteworthy, would you like to create a N&N PR for this in eclipse-platform/www.eclipse.org-eclipse.git if you haven't already? If you're not sure, I can do it for you.

[fedejeanne]

... would you like to create a N&N PR...?

My pleasure!

[fedejeanne]

@jjohnstn --> eclipse-platform/www.eclipse.org-eclipse#51