Ensure resolver tests runs unaffected by local ~/.m2 repo #5509
Conversation
| final MavenLocalRepositoryManager appCreatorLocalRepoManager = new MavenLocalRepositoryManager( | ||
| repoSystem.newLocalRepositoryManager(newSession, new LocalRepository(builder.repoHome.toString())), | ||
| Paths.get(MavenRepoInitializer.getLocalRepo(MavenRepoInitializer.getSettings()))); | ||
| builder.localRepoHome==null ? Paths.get(MavenRepoInitializer.getLocalRepo(MavenRepoInitializer.getSettings())) : builder.localRepoHome); |
There was a problem hiding this comment.
I think it should just be builder.repoHome.
There was a problem hiding this comment.
but then you are using the same dir as the remote repository and the local cache. Maven treats that differently internally.
if you as a user want it to be the same you should just do that when calling the builder.
There was a problem hiding this comment.
There is no remote repo here. This is a local repo config. Now that I'm looking into how it's implemented, the benefit of having a delegate was that installed artifacts would end up in repoHome instead of the user local one. That was useful for me at the time.
We could keep this as a feature but it shouldn't be the default behavior.
I propose to remove localRepoHome and introduce a boolean e.g. fallbackToUserRepo which if set to true (the default should be false) would trigger the current behavior, otherwise repoHome should be used as the only local repo.
There was a problem hiding this comment.
i.e. this should be the default local repo manager if repoHome isn't null: repoSystem.newLocalRepositoryManager(newSession, new LocalRepository(builder.repoHome.toString()))
There was a problem hiding this comment.
if the user wants delegation then its the current new MavenLocalRepositoryManager( repoSystem.newLocalRepositoryManager(newSession, new LocalRepository(builder.repoHome.toString())), Paths.get(MavenRepoInitializer.getLocalRepo(MavenRepoInitializer.getSettings())));
There was a problem hiding this comment.
as long as you are sure we are not by accident changing maven resolutoin logic for everything else but this one test then i'm fine - if unsure i suggest we keep the change for 1.0 absolute minimal and then in 1+ look at change the logic.
aloubyansky
force-pushed
the
ensurelocalisolation
branch
from
4a3d28e to
b835662
Compare
… repo as a fallback one for the failed artifact resolving requests in case it was setup with a custom local repo path. A new config option reTryFailedResolutionsAgainstDefaultLocalRepo was introduced on its builder to support the fallback behavior.
aloubyansky
force-pushed
the
ensurelocalisolation
branch
from
b835662 to
be59852
Compare
|
@gsmet i updated this one |
gsmet
changed the title
3 participants
When resolver tests was running they got affected by users local repo causing test assumptions to fail.
this fix explcilty set the local repo home during tests rather than listen to global default that makes behvaior different between machines that runs them.