michael-o commented on a change in pull request #12:
URL: https://github.com/apache/maven-deploy-plugin/pull/12#discussion_r546440502
##########
File path: src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java
##########
@@ -551,7 +552,88 @@ public void _testBasicDeployWithScpAsProtocol()
FileUtils.deleteDirectory( sshFile );
}
-
+
+ public void testLegacyAltDeploymentRepositoryWithDefaultLayout()
+ throws Exception
+ {
+ DeployMojo mojo = spy( new DeployMojo() );
+
+ ArtifactRepository repository = mock( ArtifactRepository.class );
+ when( mojo.createDeploymentArtifactRepository(
"altDeploymentRepository", "http://localhost";
+ ) ).thenReturn( repository );
+
+ project.setVersion( "1.0-SNAPSHOT" );
+
+ ProjectDeployerRequest pdr =
+ new ProjectDeployerRequest()
+ .setProject( project )
+ .setAltDeploymentRepository(
"altDeploymentRepository::default::http://localhost"; );
+ try
+ {
+ mojo.getDeploymentRepository( pdr );
+ fail( "Should throw: Invalid legacy syntax for repository." );
+ }
+ catch( MojoFailureException e )
+ {
+ assertEquals( e.getMessage(), "Invalid legacy syntax for
repository.");
+ assertEquals( e.getLongMessage(), "Invalid legacy syntax for
alternative repository. Use \"altDeploymentRepository::http://localhost\";
instead.");
+ }
+ }
+
+ public void testLegacyAltDeploymentRepositoryWithLegacyLayout()
+ throws Exception
+ {
+ DeployMojo mojo = spy( new DeployMojo() );
+
+ ArtifactRepository repository = mock( ArtifactRepository.class );
+ when( mojo.createDeploymentArtifactRepository(
"altDeploymentRepository", "http://localhost";
+ ) ).thenReturn( repository );
+
+ project.setVersion( "1.0-SNAPSHOT" );
+
+ ProjectDeployerRequest pdr =
+ new ProjectDeployerRequest()
+ .setProject( project )
+ .setAltDeploymentRepository(
"altDeploymentRepository::legacy::http://localhost"; );
+ try
+ {
+ mojo.getDeploymentRepository( pdr );
+ fail( "Should throw: Invalid legacy syntax for repository." );
+ }
+ catch( MojoFailureException e )
+ {
+ assertEquals( e.getMessage(), "Invalid legacy syntax for
repository.");
+ assertEquals( e.getLongMessage(), "Invalid legacy syntax for
alternative repository. Use \"altDeploymentRepository::http://localhost\";
instead.");
+ }
+ }
+
+ public void testInsaneAltDeploymentRepository()
+ throws Exception
+ {
+ DeployMojo mojo = spy( new DeployMojo() );
+
+ ArtifactRepository repository = mock( ArtifactRepository.class );
+ when( mojo.createDeploymentArtifactRepository(
"altDeploymentRepository", "http://localhost";
+ ) ).thenReturn( repository );
+
+ project.setVersion( "1.0-SNAPSHOT" );
+
+ ProjectDeployerRequest pdr =
+ new ProjectDeployerRequest()
+ .setProject( project )
+ .setAltDeploymentRepository(
"altDeploymentRepository::hey::wow::foo::http://localhost"; );
Review comment:
I consider this to be wrong too because the spilt should read:
`altDeploymentRepository`: id
`hey`: layout
`wow::foo::http://localhost`: URL
I think a valid case for a URL is `scm:svn:https://...` That's why we have
to be non-greedy.
New style is `altDeploymentRepository::scm:svn:https://...`
##########
File path: src/test/java/org/apache/maven/plugins/deploy/DeployMojoTest.java
##########
@@ -551,7 +552,88 @@ public void _testBasicDeployWithScpAsProtocol()
FileUtils.deleteDirectory( sshFile );
}
-
+
+ public void testLegacyAltDeploymentRepositoryWithDefaultLayout()
+ throws Exception
+ {
+ DeployMojo mojo = spy( new DeployMojo() );
+
+ ArtifactRepository repository = mock( ArtifactRepository.class );
+ when( mojo.createDeploymentArtifactRepository(
"altDeploymentRepository", "http://localhost";
+ ) ).thenReturn( repository );
+
+ project.setVersion( "1.0-SNAPSHOT" );
+
+ ProjectDeployerRequest pdr =
+ new ProjectDeployerRequest()
+ .setProject( project )
+ .setAltDeploymentRepository(
"altDeploymentRepository::default::http://localhost"; );
+ try
+ {
+ mojo.getDeploymentRepository( pdr );
+ fail( "Should throw: Invalid legacy syntax for repository." );
+ }
+ catch( MojoFailureException e )
+ {
+ assertEquals( e.getMessage(), "Invalid legacy syntax for
repository.");
+ assertEquals( e.getLongMessage(), "Invalid legacy syntax for
alternative repository. Use \"altDeploymentRepository::http://localhost\";
instead.");
+ }
+ }
+
+ public void testLegacyAltDeploymentRepositoryWithLegacyLayout()
+ throws Exception
+ {
+ DeployMojo mojo = spy( new DeployMojo() );
+
+ ArtifactRepository repository = mock( ArtifactRepository.class );
+ when( mojo.createDeploymentArtifactRepository(
"altDeploymentRepository", "http://localhost";
+ ) ).thenReturn( repository );
+
+ project.setVersion( "1.0-SNAPSHOT" );
+
+ ProjectDeployerRequest pdr =
+ new ProjectDeployerRequest()
+ .setProject( project )
+ .setAltDeploymentRepository(
"altDeploymentRepository::legacy::http://localhost"; );
Review comment:
I think this testcase is wrong because it should clearly say that only
`default` layout is supported. You cannot simply drop the arg from the middle
and assume that legacy layout will work. It should reject anything which is not
`default` since `createDeploymentArtifactRepository()` uses `new
DefaultRepositoryLayout()`.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
