525880: add test for preserving access rules #54
Conversation
| } | ||
|
|
||
| private void assertContainersHaveAccessRules(IJavaProject javaProject) throws JavaModelException { | ||
| List<IClasspathEntry> containerEntries = Arrays.asList(javaProject.getRawClasspath()).stream() |
There was a problem hiding this comment.
Stream.of(javaProject.getRawClasspath()) is shorter :-)
There was a problem hiding this comment.
You're right! Are you asking me to change this? If so, are there other changes you're looking for too, or is this it?
There was a problem hiding this comment.
I haven't tested the changes yet, but looking at the test, it looks like you're testing the maven container retains the access rules, globally.
I think it'd better to be more explicit, test access rules on a particular dependency. Eg. add junit and something else, add access rules to junit, check only junit has access rules after update.
There was a problem hiding this comment.
I think I'm missing something.
This change is about ensuring that access rules added to the M2E container are preserved. E.g.
<classpathentry kind="con" path="org.eclipse.m2e.MAVEN2_CLASSPATH_CONTAINER">
<attributes>
<attribute name="maven.pomderived" value="true"/>
</attributes>
<accessrules>
<accessrule kind="discouraged" pattern="**/internal/**"/>
</accessrules>
</classpathentry>
So this test is testing only the functionality that was added.
There was a problem hiding this comment.
ok my bad, as I said I haven't checked yet. But wouldn't it be better to also have a more fine grained control over each dependency?
There was a problem hiding this comment.
Perhaps, but that is not our use-case.
Out of interest I've tried adding per-dependency access rules to dependencies in the M2E container in the Eclipse UI. The UI appears to work, but no changes appear in the .classpath file, and when I re-open the "configure build path" dialog, the access rules are gone. So I'm wondering if such a thing is even possible?
There was a problem hiding this comment.
Yes it's possible this would require more complex changes to serialize the state of the maven classpath container (those changes would not be stored in .classpath, but in another file under .metadata). Let's put that one on the back burner.
There was a problem hiding this comment.
OK, sounds good.
|
Huh why does github say that I'm the committer, given that I haven't merged it yet?? |
|
I'm not sure... looks like I might have messed up the original commit: https://github.com/greensopinion/m2e-core-tests/commit/9388ed2130d4a025685022a37360e9cd97b571d4 |
Add test to verify that access rules are preserved on the Maven classpath container when a Java project is updated. Gerrit-Url: https://git.eclipse.org/r/#/c/106604/ Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=525880 Signed-off-by: David Green <david.green@tasktop.com>
|
OK, I think I've fixed the committer misattribution |
Add test to verify that access rules are preserved on the Maven
classpath container when a Java project is updated.
Gerrit-Url: https://git.eclipse.org/r/#/c/106604/
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=525880
Signed-off-by: David Green david.green@tasktop.com