elharo commented on code in PR #2458:
URL: https://github.com/apache/maven/pull/2458#discussion_r2135658006
##########
impl/maven-core/src/main/java/org/apache/maven/project/MavenProject.java:
##########
@@ -876,15 +877,11 @@ public void setArtifacts(Set<Artifact> artifacts) {
* @see #getDependencyArtifacts() to get only direct dependencies
*/
public Set<Artifact> getArtifacts() {
- if (artifacts == null) {
- if (artifactFilter == null || resolvedArtifacts == null) {
- artifacts = new LinkedHashSet<>();
- } else {
- artifacts = new LinkedHashSet<>(resolvedArtifacts.size() * 2);
- for (Artifact artifact : resolvedArtifacts) {
- if (artifactFilter.include(artifact)) {
- artifacts.add(artifact);
- }
+ if (artifactFilter != null && resolvedArtifacts != null) {
Review Comment:
This doesn't look right. It doesn't seem to have the same logic as what it
replaces when artifacts == null. I'm not sure how well tested this method is.
##########
impl/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java:
##########
@@ -215,4 +237,310 @@ void testAddDotFile() {
private void assertNoNulls(List<String> elements) {
assertFalse(elements.contains(null));
}
+
+ @Test
+ void testGetArtifacts() {
+ // Setup artifact handler
+ ArtifactHandler handler = new DefaultArtifactHandler("jar");
+
+ // Test case 1: artifacts is not null (should return cached artifacts)
+ MavenProject project1 = new MavenProject();
+ Set<Artifact> existingArtifacts = new LinkedHashSet<>();
+ existingArtifacts.add(new DefaultArtifact("group", "artifact", "1.0",
"compile", "jar", "classifier", handler));
+ project1.setArtifacts(existingArtifacts);
+ assertEquals(existingArtifacts, project1.getArtifacts());
+
+ // Test case 2: artifacts is null and either filter or resolved
artifacts is null
+ MavenProject project2 = new MavenProject();
+ assertNotNull(project2.getArtifacts());
+ assertTrue(project2.getArtifacts().isEmpty());
+
+ // Test case 3: artifacts is null but filter and resolved artifacts
exist
+ MavenProject project3 = new MavenProject();
+ Set<Artifact> resolvedArtifacts = new LinkedHashSet<>();
+ Artifact artifact1 = new DefaultArtifact("group1", "artifact1", "1.0",
"compile", "jar", null, handler);
+ Artifact artifact2 = new DefaultArtifact("group2", "artifact2", "1.0",
"test", "jar", null, handler);
+ resolvedArtifacts.add(artifact1);
+ resolvedArtifacts.add(artifact2);
+
+ project3.setResolvedArtifacts(resolvedArtifacts);
+
+ // Create a filter that includes only compile scope artifacts
+ project3.setArtifactFilter(artifact ->
"compile".equals(artifact.getScope()));
+
+ Set<Artifact> filteredArtifacts = project3.getArtifacts();
+ assertEquals(1, filteredArtifacts.size(), "Should filter artifacts
based on include filter");
+ assertTrue(filteredArtifacts.contains(artifact1), "Should include
artifact with compile scope");
+ assertFalse(filteredArtifacts.contains(artifact2), "Should exclude
artifact with test scope");
+
+ // Verify the result is cached
+ assertSame(filteredArtifacts, project3.getArtifacts(), "Should cache
the filtered artifacts");
+ }
+
+ @Test
+ void testSetAndGetAttachedArtifacts() {
+ MavenProject project = new MavenProject();
+ List<Artifact> attachedArtifacts = new java.util.ArrayList<>();
+ ArtifactHandler handler = new DefaultArtifactHandler("jar");
+ attachedArtifacts.add(new DefaultArtifact("group",
"attached-artifact", "1.0", null, "jar", "test", handler));
+ project.setAttachedArtifacts(attachedArtifacts);
+
+ assertNotNull(project.getAttachedArtifacts());
+ assertEquals(1, project.getAttachedArtifacts().size());
+ assertEquals("attached-artifact",
project.getAttachedArtifacts().get(0).getArtifactId());
+ // Change assertSame to assertEquals to compare content, not reference
Review Comment:
comment doesn't make sense, nothing changed
##########
impl/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java:
##########
@@ -215,4 +237,310 @@ void testAddDotFile() {
private void assertNoNulls(List<String> elements) {
assertFalse(elements.contains(null));
}
+
+ @Test
Review Comment:
Do any of these tests work already without changes to src/main? If so,
please add them first in a separate PR.
##########
impl/maven-core/src/test/java/org/apache/maven/project/MavenProjectTest.java:
##########
@@ -215,4 +237,310 @@ void testAddDotFile() {
private void assertNoNulls(List<String> elements) {
assertFalse(elements.contains(null));
}
+
+ @Test
+ void testGetArtifacts() {
+ // Setup artifact handler
+ ArtifactHandler handler = new DefaultArtifactHandler("jar");
+
+ // Test case 1: artifacts is not null (should return cached artifacts)
+ MavenProject project1 = new MavenProject();
+ Set<Artifact> existingArtifacts = new LinkedHashSet<>();
+ existingArtifacts.add(new DefaultArtifact("group", "artifact", "1.0",
"compile", "jar", "classifier", handler));
+ project1.setArtifacts(existingArtifacts);
+ assertEquals(existingArtifacts, project1.getArtifacts());
+
+ // Test case 2: artifacts is null and either filter or resolved
artifacts is null
Review Comment:
Each test case should be a separate method
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@maven.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
