You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@maven.apache.org by GitBox <gi...@apache.org> on 2022/05/26 13:27:00 UTC

[GitHub] [maven-scm] nielsbasjes opened a new pull request, #150: [SCM-939] Towards JUnit4 ... DRAFT !!

nielsbasjes opened a new pull request, #150:
URL: https://github.com/apache/maven-scm/pull/150

   First steps to make use of JUnit 4 so the `assume` methods actually work as expected.
   
   Some tests pass, several do not.


-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [SCM-939] Towards JUnit4 ... DRAFT !! [maven-scm]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #150:
URL: https://github.com/apache/maven-scm/pull/150#issuecomment-2042610728

   @nielsbasjes I did a few updates...


-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [SCM-939] Towards JUnit4 ... DRAFT !! [maven-scm]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #150:
URL: https://github.com/apache/maven-scm/pull/150#issuecomment-2049742397

   > Thanks @michael-o !
   
   No issue, appreciated. There might be better solutions for JUnit 4, but this is fine for me.


-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [SCM-939] Towards JUnit4 ... DRAFT !! [maven-scm]

Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on PR #150:
URL: https://github.com/apache/maven-scm/pull/150#issuecomment-2042770331

   > @nielsbasjes I did a few updates...
   
   Yes of course. Please change/copy/do what you think is best with my partial contribution.
   Feel free to take it over completely because I do not have enough understanding of several key components to finish it.


-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [SCM-939] Towards JUnit4 ... DRAFT !! [maven-scm]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o closed pull request #150: [SCM-939] Towards JUnit4 ... DRAFT !!
URL: https://github.com/apache/maven-scm/pull/150


-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] nielsbasjes commented on a diff in pull request #150: [SCM-939] Towards JUnit4 ... DRAFT !!

Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #150:
URL: https://github.com/apache/maven-scm/pull/150#discussion_r886035241


##########
maven-scm-plugin/src/test/java/org/apache/maven/scm/plugin/BranchMojoTest.java:
##########
@@ -79,36 +85,33 @@ protected void setUp()
         checkoutMojo.execute();
     }
 
+    @Test
     public void testBranch()
         throws Exception
     {
-        if ( !ScmTestCase.isSystemCmd( SvnScmTestUtils.SVN_COMMAND_LINE ) )
-        {
-            ScmTestCase.printSystemCmdUnavail( SvnScmTestUtils.SVN_COMMAND_LINE, getName() );
-            return;
-        }
+        checkScmPresence( SvnScmTestUtils.SVNADMIN_COMMAND_LINE, "testBranch" );
 
         BranchMojo mojo =
-            (BranchMojo) lookupMojo( "branch", getTestFile( "src/test/resources/mojos/branch/branch.xml" ) );
+            (BranchMojo) lookupMojo( "branch", PlexusJUnit4TestSupport.getTestFile( "src/test/resources/mojos/branch/branch.xml" ) );

Review Comment:
   Because this is a very incomplete draft.
   My first goal was to make it work an then restructure to make it clean.
   I have note been able to reach this first goal yet.



-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [SCM-939] Towards JUnit4 ... DRAFT !! [maven-scm]

Posted by "michael-o (via GitHub)" <gi...@apache.org>.
michael-o commented on PR #150:
URL: https://github.com/apache/maven-scm/pull/150#issuecomment-2042791182

   > > @nielsbasjes I did a few updates...
   > 
   > Yes of course. Please change/copy/do what you think is best with my partial contribution. Feel free to take it over completely because I do not have enough understanding of several key components to finish it.
   
   For me, this looks decent now, but I want to make a few more checks. @kwin Do you want to review?


-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] michael-o commented on pull request #150: [SCM-939] Towards JUnit4 ... DRAFT !!

Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #150:
URL: https://github.com/apache/maven-scm/pull/150#issuecomment-1142545754

   @cstamas WDYT?


-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] [SCM-939] Towards JUnit4 ... DRAFT !! [maven-scm]

Posted by "nielsbasjes (via GitHub)" <gi...@apache.org>.
nielsbasjes commented on PR #150:
URL: https://github.com/apache/maven-scm/pull/150#issuecomment-2049682493

   Thanks @michael-o !


-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] michael-o commented on a diff in pull request #150: [SCM-939] Towards JUnit4 ... DRAFT !!

Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #150:
URL: https://github.com/apache/maven-scm/pull/150#discussion_r886032110


##########
maven-scm-plugin/src/test/java/org/apache/maven/scm/plugin/BranchMojoTest.java:
##########
@@ -79,36 +85,33 @@ protected void setUp()
         checkoutMojo.execute();
     }
 
+    @Test
     public void testBranch()
         throws Exception
     {
-        if ( !ScmTestCase.isSystemCmd( SvnScmTestUtils.SVN_COMMAND_LINE ) )
-        {
-            ScmTestCase.printSystemCmdUnavail( SvnScmTestUtils.SVN_COMMAND_LINE, getName() );
-            return;
-        }
+        checkScmPresence( SvnScmTestUtils.SVNADMIN_COMMAND_LINE, "testBranch" );
 
         BranchMojo mojo =
-            (BranchMojo) lookupMojo( "branch", getTestFile( "src/test/resources/mojos/branch/branch.xml" ) );
+            (BranchMojo) lookupMojo( "branch", PlexusJUnit4TestSupport.getTestFile( "src/test/resources/mojos/branch/branch.xml" ) );

Review Comment:
   Why not have the method in the parent class thorugh delegation?



-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [maven-scm] nielsbasjes commented on a diff in pull request #150: [SCM-939] Towards JUnit4 ... DRAFT !!

Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #150:
URL: https://github.com/apache/maven-scm/pull/150#discussion_r886110692


##########
maven-scm-plugin/src/test/java/org/apache/maven/scm/plugin/BranchMojoTest.java:
##########
@@ -79,36 +85,33 @@ protected void setUp()
         checkoutMojo.execute();
     }
 
+    @Test
     public void testBranch()
         throws Exception
     {
-        if ( !ScmTestCase.isSystemCmd( SvnScmTestUtils.SVN_COMMAND_LINE ) )
-        {
-            ScmTestCase.printSystemCmdUnavail( SvnScmTestUtils.SVN_COMMAND_LINE, getName() );
-            return;
-        }
+        checkScmPresence( SvnScmTestUtils.SVNADMIN_COMMAND_LINE, "testBranch" );
 
         BranchMojo mojo =
-            (BranchMojo) lookupMojo( "branch", getTestFile( "src/test/resources/mojos/branch/branch.xml" ) );
+            (BranchMojo) lookupMojo( "branch", PlexusJUnit4TestSupport.getTestFile( "src/test/resources/mojos/branch/branch.xml" ) );

Review Comment:
   So just to be clear. I'm totally fine if you guys pick this one up.
   I'm stuck here.



-- 
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-unsubscribe@maven.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org