Import test apps on worker threads; Delete unused test files #436
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
scottkurz
force-pushed
the
import-gradle-test-app-on-worker-thread
branch
2 times, most recently
from
503a450
to
431030e
Compare
Signed-off-by: Scott Kurz <[email protected]>
Signed-off-by: Scott Kurz <[email protected]>
scottkurz
force-pushed
the
import-gradle-test-app-on-worker-thread
branch
from
431030e
to
0935ae2
Compare
scottkurz
changed the title
awisniew90
approved these changes
Sep 15, 2023
| // Get the list of projects to install. | ||
| MavenModelManager modelManager = MavenPlugin.getMavenModelManager(); | ||
|
|
||
| for (String folder : folders) { |
There was a problem hiding this comment.
Is there a reason for iterating over each folder rather than passing the folders list to the LocalProjectScanner?
There was a problem hiding this comment.
I was just grasping and wondering if it would be more or less likely to hit something like the #375 issue.
It did not seem to eliminate some of the error messages I noted above so far...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A couple things going on here.
First, I got some failures on the Gradle setup, messages hinting at a deadlock. It seemed to suggest to me that maybe we shouldn't be doing the import from the main/UI thread. Reading the Gradle javadoc, it seemed the import stuff was long-running.
I decided to move this out into a non-UI thread. Since then I have not seen this deadlock issue (which isn't exactly proof that this change fixed it, but I'll take it as a good sign).
Second, I decided to do the same thing with Maven. As I was making this change, I was paying more attention to import, and I noticed some error messages that, however, did not seem to fail the tests.
Here's a sample snippet:
I also noticed some issues setting up the DS componentry, like:
I'm not sure how relevant these are. They seem to be present before and after this change here... and so ultimately without consequence.
I had a thought that I wondered if there might be some relevance to the #375 issue, but I guess there's still no reason to think so.
Finally, I simply deleted some old test parts that I think are now unused. I believe we deleted the test code that was trying to dynamically switch btw wrapper / non-wrapper and did the equivalent more statically (though I don't completely trust my memory, but I kind of recall this).