Use Agent for parsing Git repo names (CODY-3132) #2257
Conversation
| @@ -128,7 +132,25 @@ private static String doReplacements( | |||
|
|
|||
| if (vcsType == VCSType.GIT && repository != null) { | |||
| String cloneURL = GitUtil.getRemoteRepoUrl((GitRepository) repository, project); | |||
| return convertGitCloneURLToCodebaseNameOrError(cloneURL).getValue(); | |||
| AtomicReference<String> repoURl = new AtomicReference<>(); | |||
There was a problem hiding this comment.
This isn't right. withAgent runs the callback on a separate thread, so you will often--but not always--have null in repoURl.
You need to:
- Make
getRemoteRepoUrlreturn a CompletableFuture<String?> - Convert the callers to handle an asynchronous result. This may take care and attention to not block EDT, not starve a threadpool, ensure that if you started on EDT you continue on EDT if the caller requires EDT, etc.
- Convert the method body to handle the asynchronous result: Create the future, run the code with Agent, complete the result in the callback.
By the way, let's pay attention to details like repoURL or repoUrl but not repoURl
There was a problem hiding this comment.
Ah for some reason I thought it was blocking. I've updated it in this commit. Do you know if there are any unit tests of agent integration? Would this be a good use case for an integration test?
| } | ||
|
|
||
| // Handle Azure DevOps URLs | ||
| if (uri.host?.contains("dev.azure") == true && !uri.path.isNullOrEmpty()) { |
There was a problem hiding this comment.
Are all of these cases handled in unit tests on the Agent side?
There was a problem hiding this comment.
Yes in this file. In fact it looks like the tests here were carbon copied from there.
|
This function was in agent long time ago, and was copied to jetbrains because it turned out to be problematic. Also actions like |
|
@pkukielka @dominiccooney agreed on the testing required. Do either of you have a suggestion of how to test this? Do we have any tests for agent communication? Should this be an integration test? |
CODY-3132
There was a duplication of effort around the convertGitURLToCodebaseName as it was implemented in the agent and here. There was even already an agent method for this, but it was unused. Thus when fixing the above on the agent side, I thought it made sense to switch it here.
Test plan
I'm not sure the best way to test this, so input would be appreciated! Do we have tests for agent integration?