Copilot commented on code in PR #837:
URL: https://github.com/apache/maven-wagon/pull/837#discussion_r2616919330
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/AbstractWagonTest.java:
##########
@@ -277,28 +279,27 @@ protected void openConnectionInternal() throws
ConnectionException, Authenticati
wagon.connect(repository);
fail();
} finally {
- verify(sessionListener);
-
+ verify(sessionListener).sessionOpening(any(SessionEvent.class));
+
verify(sessionListener).sessionConnectionRefused(any(SessionEvent.class));
assertEquals(repository, wagon.getRepository());
}
}
- public void testSessionCloseEvents() throws Exception {
- sessionListener.sessionDisconnecting(anyObject(SessionEvent.class));
- sessionListener.sessionDisconnected(anyObject(SessionEvent.class));
- replay(sessionListener);
+ @Test
+ void sessionCloseEvents() throws Exception {
+ sessionListener.sessionDisconnecting(any(SessionEvent.class));
+ sessionListener.sessionDisconnected(any(SessionEvent.class));
Review Comment:
These method calls on lines 290-291 are not valid Mockito syntax and don't
serve any purpose. In Mockito, you don't pre-record expectations like in
EasyMock. These lines should be removed as they don't configure the mock
behavior and the actual verification is correctly done after the method call on
lines 295-296.
```suggestion
```
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/AbstractWagonTest.java:
##########
@@ -308,106 +309,78 @@ protected void closeConnection() throws
ConnectionException {
try {
wagon.disconnect();
fail();
- } catch (ConnectionException e) {
- assertTrue(true);
+ } catch (ConnectionException ignored) {
} finally {
- verify(sessionListener);
+
verify(sessionListener).sessionDisconnecting(any(SessionEvent.class));
+ verify(sessionListener).sessionError(any(SessionEvent.class));
+ verify(sessionListener,
never()).sessionDisconnected(any(SessionEvent.class));
}
}
- public void testGetTransferEvents() throws Exception {
- transferListener.debug("fetch debug message");
- transferListener.transferInitiated(anyObject(TransferEvent.class));
- transferListener.transferStarted(anyObject(TransferEvent.class));
- transferListener.debug(anyString());
- expectLastCall().anyTimes();
- transferListener.transferProgress(anyObject(TransferEvent.class),
anyObject(byte[].class), anyInt());
- expectLastCall().times(5);
- transferListener.transferCompleted(anyObject(TransferEvent.class));
- replay(transferListener);
-
- wagon.fireTransferDebug("fetch debug message");
-
- Repository repository = new Repository();
+ @Test
+ void getTransferEvents() throws Exception {
+ Repository repository = new Repository("fake", "http://fake");
wagon.connect(repository);
wagon.get(artifact, destination);
- verify(transferListener);
- }
-
- public void testGetError() throws Exception {
- transferListener.transferInitiated(anyObject(TransferEvent.class));
- transferListener.transferStarted(anyObject(TransferEvent.class));
- transferListener.debug(anyString());
- expectLastCall().anyTimes();
- transferListener.transferError(anyObject(TransferEvent.class));
- replay(transferListener);
+ wagon.fireTransferDebug("fetch debug message");
+ verify(transferListener).debug("fetch debug message");
Review Comment:
The debug message is fired after the transfer completes, which changes the
test behavior from the original. The `fireTransferDebug` call on line 327
should be moved before the `wagon.get()` call on line 325 to match the original
test logic where the debug message was fired before the transfer operation.
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/events/TransferEventTest.java:
##########
@@ -144,7 +114,7 @@ public void testConstantValueConflict() {
final String msg = "Value confict at [i,j]=[" + i + "," + j +
"]";
- assertTrue(msg, values[i] != values[j]);
+ assertNotSame(values[i], values[j], msg);
Review Comment:
The assertion logic is incorrect. The method `assertNotSame` checks if two
object references are not the same, but the code is comparing primitive int
values. Use `assertNotEquals` instead to compare the values themselves.
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/repository/RepositoryTest.java:
##########
@@ -94,7 +86,8 @@ public void testRepositoryProperties() throws Exception {
assertEquals("http://www.ibiblio.org", repository.getUrl());
}
- public void testIPv6() {
+ @Test
+ void iPv6() {
Review Comment:
The test method name `iPv6` does not follow standard Java naming
conventions. In JUnit 5, test methods should use camelCase starting with a
lowercase letter. Consider renaming to `ipV6` to be consistent with typical
Java naming patterns.
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/events/SessionEventSupportTest.java:
##########
@@ -18,227 +18,168 @@
*/
package org.apache.maven.wagon.events;
-import junit.framework.TestCase;
import org.apache.maven.wagon.Wagon;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
-import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.createNiceMock;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.verify;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
/**
* @author <a href="[email protected]">Michal Maczka</a>
*
*/
-public class SessionEventSupportTest extends TestCase {
+class SessionEventSupportTest {
- private SessionEventSupport eventSupport;
+ private final SessionEventSupport eventSupport = new SessionEventSupport();
private Wagon wagon;
- /**
- * @see junit.framework.TestCase#setUp()
- */
- protected void setUp() throws Exception {
- super.setUp();
-
- eventSupport = new SessionEventSupport();
-
+ @BeforeEach
+ void setUp() {
// TODO: actually test it gets called?
- wagon = createNiceMock(Wagon.class);
+ wagon = mock(Wagon.class);
}
- public void testSessionListenerRegistration() {
- SessionListener mock1 = createMock(SessionListener.class);
-
+ @Test
+ void sessionListenerRegistration() {
+ SessionListener mock1 = mock(SessionListener.class);
eventSupport.addSessionListener(mock1);
-
assertTrue(eventSupport.hasSessionListener(mock1));
- SessionListener mock2 = createMock(SessionListener.class);
-
+ SessionListener mock2 = mock(SessionListener.class);
assertFalse(eventSupport.hasSessionListener(mock2));
eventSupport.addSessionListener(mock2);
-
assertTrue(eventSupport.hasSessionListener(mock1));
-
assertTrue(eventSupport.hasSessionListener(mock2));
eventSupport.removeSessionListener(mock2);
-
assertTrue(eventSupport.hasSessionListener(mock1));
-
assertFalse(eventSupport.hasSessionListener(mock2));
eventSupport.removeSessionListener(mock1);
-
assertFalse(eventSupport.hasSessionListener(mock1));
}
- public void testFireSessionDisconnected() {
- SessionListener mock1 = createMock(SessionListener.class);
-
+ @Test
+ void fireSessionDisconnected() {
+ SessionListener mock1 = mock(SessionListener.class);
eventSupport.addSessionListener(mock1);
- SessionListener mock2 = createMock(SessionListener.class);
-
+ SessionListener mock2 = mock(SessionListener.class);
eventSupport.addSessionListener(mock2);
- final SessionEvent event = new SessionEvent(wagon, 1);
-
- mock1.sessionDisconnected(event);
- mock2.sessionDisconnected(event);
-
- replay(mock1, mock2);
-
+ SessionEvent event = new SessionEvent(wagon, 1);
eventSupport.fireSessionDisconnected(event);
- verify(mock1, mock2);
+ verify(mock1).sessionDisconnected(event);
+ verify(mock2).sessionDisconnected(event);
}
- public void testFireSessionDisconneting() {
- SessionListener mock1 = createMock(SessionListener.class);
-
+ @Test
+ void fireSessionDisconneting() {
+ SessionListener mock1 = mock(SessionListener.class);
eventSupport.addSessionListener(mock1);
- SessionListener mock2 = createMock(SessionListener.class);
-
+ SessionListener mock2 = mock(SessionListener.class);
eventSupport.addSessionListener(mock2);
- final SessionEvent event = new SessionEvent(wagon, 1);
+ SessionEvent event = new SessionEvent(wagon, 1);
+ eventSupport.fireSessionDisconnecting(event);
mock1.sessionDisconnecting(event);
mock2.sessionDisconnecting(event);
Review Comment:
The test verifies mock calls but the order of operations is incorrect. The
verify statements should be called after the event firing has completed.
Currently, lines 93-94 verify before the event is fired on line 91.
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/PathUtilsTest.java:
##########
@@ -268,7 +289,8 @@ public void testIpV4() {
"/oo/rest/users");
}
- public void testIPv6() {
+ @Test
+ void iPv6() {
Review Comment:
The test method name `iPv6` does not follow standard Java naming
conventions. In JUnit 5, test methods should use camelCase starting with a
lowercase letter. Consider renaming to `ipV6` to be consistent with typical
Java naming patterns.
```suggestion
void ipV6() {
```
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/events/SessionEventTest.java:
##########
@@ -113,7 +108,7 @@ public void testConstantValueConflict() {
for (int j = i + 1; j < values.length; j++) {
final String msg = "Value confict at [i,j]=[" + i + "," + j +
"]";
- assertTrue(msg, values[i] != values[j]);
+ assertNotSame(values[i], values[j], msg);
Review Comment:
The assertion logic is incorrect. The method `assertNotSame` checks if two
object references are not the same, but the code is comparing primitive int
values. Use `assertNotEquals` instead to compare the values themselves.
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/proxy/ProxyInfoUtilsTest.java:
##########
@@ -18,29 +18,23 @@
*/
package org.apache.maven.wagon.proxy;
-import junit.framework.TestCase;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
/**
* @author <a href="mailto:[email protected]">Thomas Champagne</a>
*/
-public class ProxyInfoUtilsTest extends TestCase {
- public ProxyInfoUtilsTest(final String name) {
- super(name);
- }
-
- public void setUp() throws Exception {
- super.setUp();
- }
-
- public void tearDown() throws Exception {
- super.tearDown();
- }
+public class ProxyInfoUtilsTest {
- public void testValidateNonProxyHostsWithNullProxy() {
- assertFalse("www.ibiblio.org", ProxyUtils.validateNonProxyHosts(null,
"maven.apache.org"));
+ @Test
+ void validateNonProxyHostsWithNullProxy() {
+ assertFalse(ProxyUtils.validateNonProxyHosts(null,
"maven.apache.org"), "www.ibiblio.org");
Review Comment:
The assertion order is swapped. In JUnit 5, the actual value should be the
first parameter and the expected value (the message) should be the second
parameter. This should be `assertFalse(ProxyUtils.validateNonProxyHosts(null,
"maven.apache.org"), "www.ibiblio.org")` to match the migration pattern used
elsewhere in the codebase.
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/AbstractWagonTest.java:
##########
@@ -447,61 +424,62 @@ public void testRepositoryUserNameNotGivenInCredentials()
throws ConnectionExcep
assertEquals("password", authenticationInfo.getPassword());
}
- public void testConnectNullRepository() throws ConnectionException,
AuthenticationException {
+ @Test
+ void connectNullRepository() throws Exception {
try {
wagon.connect(null);
fail();
- } catch (NullPointerException e) {
- assertTrue(true);
+ } catch (NullPointerException ignored) {
}
}
- public void testPostProcessListeners() throws TransferFailedException,
IOException {
+ @Test
+ void postProcessListeners() throws Exception {
File tempFile = File.createTempFile("wagon", "tmp");
tempFile.deleteOnExit();
String content = "content";
- FileUtils.fileWrite(tempFile.getAbsolutePath(), content);
+ Files.write(Paths.get(tempFile.getAbsolutePath()), content.getBytes());
Resource resource = new Resource("resource");
- transferListener.transferInitiated(anyObject(TransferEvent.class));
- transferListener.transferStarted(anyObject(TransferEvent.class));
+ transferListener.transferInitiated(any(TransferEvent.class));
+ transferListener.transferStarted(any(TransferEvent.class));
TransferEvent event =
new TransferEvent(wagon, resource,
TransferEvent.TRANSFER_PROGRESS, TransferEvent.REQUEST_PUT);
event.setLocalFile(tempFile);
- transferListener.transferProgress(eq(event), anyObject(byte[].class),
eq(content.length()));
- ProgressAnswer answer = new ProgressAnswer();
- expectLastCall().andAnswer(answer);
- transferListener.transferCompleted(anyObject(TransferEvent.class));
- replay(transferListener);
+ transferListener.transferProgress(eq(event), any(byte[].class),
eq(content.length()));
+ // ProgressAnswer answer = new ProgressAnswer();
+ // expectLastCall().thenAnswer(answer);
+ transferListener.transferCompleted(any(TransferEvent.class));
wagon.postProcessListeners(resource, tempFile,
TransferEvent.REQUEST_PUT);
- assertEquals(content.length(), answer.getSize());
- assertEquals(new String(content.getBytes()), new
String(answer.getBytes()));
+ // assertEquals(content.length(), answer.getSize());
+ // assertEquals(new String(content.getBytes()), new
String(answer.getBytes()));
tempFile.delete();
}
-
- static final class ProgressAnswer implements IAnswer {
- private ByteArrayOutputStream baos = new ByteArrayOutputStream();
-
- private int size;
-
- public Object answer() throws Throwable {
- byte[] buffer = (byte[]) getCurrentArguments()[1];
- int length = (Integer) getCurrentArguments()[2];
- baos.write(buffer, 0, length);
- size += length;
- return null;
- }
-
- public int getSize() {
- return size;
- }
-
- public byte[] getBytes() {
- return baos.toByteArray();
- }
- }
+ //
+ // static final class ProgressAnswer implements IAnswer {
+ // private final ByteArrayOutputStream baos = new
ByteArrayOutputStream();
+ //
+ // private int size;
+ //
+ // @Override
+ // public Object answer() {
+ // byte[] buffer = (byte[]) getCurrentArguments()[1];
+ // int length = (Integer) getCurrentArguments()[2];
+ // baos.write(buffer, 0, length);
+ // size += length;
+ // return null;
+ // }
+ //
+ // public int getSize() {
+ // return size;
+ // }
+ //
+ // public byte[] getBytes() {
+ // return baos.toByteArray();
+ // }
+ // }
Review Comment:
Commented-out code should be removed instead of being left in the codebase.
If this functionality is no longer needed, delete these lines completely. If it
will be needed in the future, it should be tracked in an issue tracker rather
than kept as dead code.
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/events/SessionEventSupportTest.java:
##########
@@ -18,227 +18,168 @@
*/
package org.apache.maven.wagon.events;
-import junit.framework.TestCase;
import org.apache.maven.wagon.Wagon;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
-import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.createNiceMock;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.verify;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
/**
* @author <a href="[email protected]">Michal Maczka</a>
*
*/
-public class SessionEventSupportTest extends TestCase {
+class SessionEventSupportTest {
- private SessionEventSupport eventSupport;
+ private final SessionEventSupport eventSupport = new SessionEventSupport();
private Wagon wagon;
- /**
- * @see junit.framework.TestCase#setUp()
- */
- protected void setUp() throws Exception {
- super.setUp();
-
- eventSupport = new SessionEventSupport();
-
+ @BeforeEach
+ void setUp() {
// TODO: actually test it gets called?
- wagon = createNiceMock(Wagon.class);
+ wagon = mock(Wagon.class);
}
- public void testSessionListenerRegistration() {
- SessionListener mock1 = createMock(SessionListener.class);
-
+ @Test
+ void sessionListenerRegistration() {
+ SessionListener mock1 = mock(SessionListener.class);
eventSupport.addSessionListener(mock1);
-
assertTrue(eventSupport.hasSessionListener(mock1));
- SessionListener mock2 = createMock(SessionListener.class);
-
+ SessionListener mock2 = mock(SessionListener.class);
assertFalse(eventSupport.hasSessionListener(mock2));
eventSupport.addSessionListener(mock2);
-
assertTrue(eventSupport.hasSessionListener(mock1));
-
assertTrue(eventSupport.hasSessionListener(mock2));
eventSupport.removeSessionListener(mock2);
-
assertTrue(eventSupport.hasSessionListener(mock1));
-
assertFalse(eventSupport.hasSessionListener(mock2));
eventSupport.removeSessionListener(mock1);
-
assertFalse(eventSupport.hasSessionListener(mock1));
}
- public void testFireSessionDisconnected() {
- SessionListener mock1 = createMock(SessionListener.class);
-
+ @Test
+ void fireSessionDisconnected() {
+ SessionListener mock1 = mock(SessionListener.class);
eventSupport.addSessionListener(mock1);
- SessionListener mock2 = createMock(SessionListener.class);
-
+ SessionListener mock2 = mock(SessionListener.class);
eventSupport.addSessionListener(mock2);
- final SessionEvent event = new SessionEvent(wagon, 1);
-
- mock1.sessionDisconnected(event);
- mock2.sessionDisconnected(event);
-
- replay(mock1, mock2);
-
+ SessionEvent event = new SessionEvent(wagon, 1);
eventSupport.fireSessionDisconnected(event);
- verify(mock1, mock2);
+ verify(mock1).sessionDisconnected(event);
+ verify(mock2).sessionDisconnected(event);
}
- public void testFireSessionDisconneting() {
- SessionListener mock1 = createMock(SessionListener.class);
-
+ @Test
+ void fireSessionDisconneting() {
+ SessionListener mock1 = mock(SessionListener.class);
eventSupport.addSessionListener(mock1);
- SessionListener mock2 = createMock(SessionListener.class);
-
+ SessionListener mock2 = mock(SessionListener.class);
eventSupport.addSessionListener(mock2);
- final SessionEvent event = new SessionEvent(wagon, 1);
+ SessionEvent event = new SessionEvent(wagon, 1);
+ eventSupport.fireSessionDisconnecting(event);
mock1.sessionDisconnecting(event);
mock2.sessionDisconnecting(event);
Review Comment:
The verification is using incorrect Mockito syntax. With Mockito, the mock
expectations should not be recorded before the actual call. The
`mock1.sessionDisconnecting(event)` and `mock2.sessionDisconnecting(event)`
statements on lines 93-94 are being executed as actual method calls, not as
verification steps. These lines should be removed, and only the `verify` calls
after the event firing should remain.
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/AbstractWagonTest.java:
##########
@@ -308,106 +309,78 @@ protected void closeConnection() throws
ConnectionException {
try {
wagon.disconnect();
fail();
- } catch (ConnectionException e) {
- assertTrue(true);
+ } catch (ConnectionException ignored) {
} finally {
- verify(sessionListener);
+
verify(sessionListener).sessionDisconnecting(any(SessionEvent.class));
+ verify(sessionListener).sessionError(any(SessionEvent.class));
+ verify(sessionListener,
never()).sessionDisconnected(any(SessionEvent.class));
}
}
- public void testGetTransferEvents() throws Exception {
- transferListener.debug("fetch debug message");
- transferListener.transferInitiated(anyObject(TransferEvent.class));
- transferListener.transferStarted(anyObject(TransferEvent.class));
- transferListener.debug(anyString());
- expectLastCall().anyTimes();
- transferListener.transferProgress(anyObject(TransferEvent.class),
anyObject(byte[].class), anyInt());
- expectLastCall().times(5);
- transferListener.transferCompleted(anyObject(TransferEvent.class));
- replay(transferListener);
-
- wagon.fireTransferDebug("fetch debug message");
-
- Repository repository = new Repository();
+ @Test
+ void getTransferEvents() throws Exception {
+ Repository repository = new Repository("fake", "http://fake");
wagon.connect(repository);
wagon.get(artifact, destination);
- verify(transferListener);
- }
-
- public void testGetError() throws Exception {
- transferListener.transferInitiated(anyObject(TransferEvent.class));
- transferListener.transferStarted(anyObject(TransferEvent.class));
- transferListener.debug(anyString());
- expectLastCall().anyTimes();
- transferListener.transferError(anyObject(TransferEvent.class));
- replay(transferListener);
+ wagon.fireTransferDebug("fetch debug message");
+ verify(transferListener).debug("fetch debug message");
- try {
- Repository repository = new Repository();
+ verify(transferListener).transferInitiated(any(TransferEvent.class));
+ verify(transferListener).transferStarted(any(TransferEvent.class));
+ verify(transferListener, atLeastOnce()).debug(anyString());
+ verify(transferListener,
atLeastOnce()).transferProgress(any(TransferEvent.class), any(byte[].class),
anyInt());
+ verify(transferListener).transferCompleted(any(TransferEvent.class));
+ }
+ @Test
+ void getError() {
+ assertThrows(TransferFailedException.class, () -> {
+ Repository repository = new Repository("fake", "http://fake");
WagonMock wagon = new WagonMock(true);
-
wagon.addTransferListener(transferListener);
wagon.connect(repository);
wagon.get(artifact, destination);
+ });
- fail("Transfer error was expected during deploy");
- } catch (TransferFailedException expected) {
- assertTrue(true);
- }
-
- verify(transferListener);
+ verify(transferListener).transferInitiated(any(TransferEvent.class));
+ verify(transferListener).transferStarted(any(TransferEvent.class));
+ verify(transferListener).debug(anyString());
+ verify(transferListener).transferError(any(TransferEvent.class));
}
- public void testPutTransferEvents()
- throws ConnectionException, AuthenticationException,
ResourceDoesNotExistException, TransferFailedException,
- AuthorizationException {
- transferListener.debug("deploy debug message");
- transferListener.transferInitiated(anyObject(TransferEvent.class));
- transferListener.transferStarted(anyObject(TransferEvent.class));
- transferListener.transferProgress(anyObject(TransferEvent.class),
anyObject(byte[].class), anyInt());
- transferListener.transferCompleted(anyObject(TransferEvent.class));
- replay(transferListener);
-
+ @Test
+ void putTransferEvents() throws Exception {
wagon.fireTransferDebug("deploy debug message");
- Repository repository = new Repository();
-
+ Repository repository = new Repository("fake", "http://fake");
wagon.connect(repository);
-
wagon.put(source, artifact);
- verify(transferListener);
+ verify(transferListener).transferInitiated(any(TransferEvent.class));
+ verify(transferListener).transferStarted(any(TransferEvent.class));
+ verify(transferListener).transferProgress(any(TransferEvent.class),
any(byte[].class), anyInt());
+ verify(transferListener).transferCompleted(any(TransferEvent.class));
+ verify(transferListener).debug("deploy debug message");
}
- public void testStreamShutdown() {
- IOUtil.close((InputStream) null);
-
- IOUtil.close((OutputStream) null);
-
- InputStreamMock inputStream = new InputStreamMock();
-
- assertFalse(inputStream.isClosed());
-
- IOUtil.close(inputStream);
-
- assertTrue(inputStream.isClosed());
-
- OutputStreamMock outputStream = new OutputStreamMock();
-
- assertFalse(outputStream.isClosed());
-
- IOUtil.close(outputStream);
+ @Test
+ void streamShutdown() {
+ try (InputStreamMock inputStream = new InputStreamMock()) {
+ assertFalse(inputStream.isClosed());
+ }
- assertTrue(outputStream.isClosed());
+ try (OutputStreamMock outputStream = new OutputStreamMock()) {
+ assertFalse(outputStream.isClosed());
+ }
Review Comment:
The try-with-resources statement is not properly handling the AutoCloseable
resources. When using try-with-resources with InputStreamMock and
OutputStreamMock, the resources are automatically closed at the end of the
block, defeating the purpose of testing whether they are initially not closed.
The test logic should be restructured to test the closed state after the
try-with-resources block completes.
##########
wagon-provider-api/src/test/java/org/apache/maven/wagon/StreamWagonTest.java:
##########
@@ -18,154 +18,170 @@
*/
package org.apache.maven.wagon;
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.nio.file.Files;
+import java.nio.file.Paths;
import java.text.SimpleDateFormat;
-import junit.framework.TestCase;
import org.apache.maven.wagon.authentication.AuthenticationException;
import org.apache.maven.wagon.authorization.AuthorizationException;
import org.apache.maven.wagon.events.TransferEvent;
import org.apache.maven.wagon.events.TransferListener;
import org.apache.maven.wagon.repository.Repository;
import org.apache.maven.wagon.resource.Resource;
-import org.codehaus.plexus.util.FileUtils;
-import org.codehaus.plexus.util.StringInputStream;
-import org.codehaus.plexus.util.StringOutputStream;
+import org.junit.jupiter.api.Test;
-import static org.easymock.EasyMock.anyObject;
-import static org.easymock.EasyMock.createMock;
-import static org.easymock.EasyMock.replay;
-import static org.easymock.EasyMock.verify;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.fail;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+
+class StreamWagonTest {
-public class StreamWagonTest extends TestCase {
private static class TestWagon extends StreamWagon {
- public void closeConnection() throws ConnectionException {}
+ @Override
+ public void closeConnection() {}
+ @Override
public void fillInputData(InputData inputData)
throws TransferFailedException, ResourceDoesNotExistException,
AuthorizationException {}
+ @Override
public void fillOutputData(OutputData outputData) throws
TransferFailedException {}
- protected void openConnectionInternal() throws ConnectionException,
AuthenticationException {}
+ @Override
+ protected void openConnectionInternal() {}
}
- private Repository repository = new Repository("id", "url");
+ private final Repository repository = new Repository("id", "url");
+
+ @Test
+ void nullInputStream() throws Exception {
- public void testNullInputStream() throws Exception {
StreamingWagon wagon = new TestWagon() {
+ @Override
public void fillInputData(InputData inputData) {
inputData.setInputStream(null);
}
};
- TransferListener listener = createMock(TransferListener.class);
- listener.transferInitiated(anyObject(TransferEvent.class));
+ TransferListener listener = mock(TransferListener.class);
+ listener.transferInitiated(any(TransferEvent.class));
+
TransferEvent transferEvent = new TransferEvent(
wagon, new Resource("resource"), new
TransferFailedException(""), TransferEvent.REQUEST_GET);
listener.transferError(transferEvent);
- replay(listener);
wagon.connect(repository);
wagon.addTransferListener(listener);
- try {
- wagon.getToStream("resource", new StringOutputStream());
+
+ try (OutputStream os = new ByteArrayOutputStream()) {
+ wagon.getToStream("resource", os);
fail();
} catch (TransferFailedException e) {
- assertTrue(true);
+ assertEquals("url - Could not open input stream for resource:
'resource'", e.getMessage());
Review Comment:
The assertion is missing the validation of the error message. The previous
test had an assertion `assertTrue(true)` in the catch block which, while not
ideal, indicated the exception was expected. The new code checks the exception
message, but line 93 should verify the actual error message matches what's
expected from the TransferFailedException, or at minimum assert that the
exception is not null.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]