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/20 07:53:29 UTC
[GitHub] [maven-scm] nielsbasjes opened a new pull request, #140: [SCM-981] Add run-its profile and fix broken tests
nielsbasjes opened a new pull request, #140:
URL: https://github.com/apache/maven-scm/pull/140
Most of the tests are never run.
This patch:
- adds a tck-hg profile to run the HG specific tck tests.
- adds a run-its profile to run the all provider tck tests.
- Fixes the tests that fail. Those were all very small fixes so I include them here.
Now `mvn clean verify -Prun-its` runs all tck tests and passes (on my machine).
--
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 pull request #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1132861015
Hmmm, this assumeTrue makes the test still fail eventhough the documentation indicates they should be skipped instead:
```
[ERROR] Errors:
[ERROR] JGitBranchCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitChangeLogCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitChangeLogCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitChangeLogCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitChangeLogCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitChangeLogCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitChangeLogCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitChangeLogCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitChangeLogCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitChangeLogCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitCheckInCommandCommitterAuthorTckTest.setUp:60->ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitCheckInCommandCommitterAuthorTckTest.setUp:60->ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitCheckInCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitCheckInCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitCheckOutCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitDiffCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitRemoteInfoCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitStatusCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitTagCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[ERROR] JGitUntagCommandTckTest>ScmTckTestCase.setUp:136->ScmTckTestCase.checkScmPresence:124 » AssumptionViolated
[INFO]
[ERROR] Tests run: 20, Failures: 0, Errors: 20, Skipped: 0
```
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r879788525
##########
maven-scm-providers/maven-scm-provider-hg/src/main/java/org/apache/maven/scm/provider/hg/command/changelog/HgChangeLogConsumer.java:
##########
@@ -148,7 +148,6 @@ else if ( line.startsWith( MESSAGE_TOKEN ) )
{
StringBuilder comment = new StringBuilder( currentChange.getComment() );
comment.append( line );
- comment.append( '\n' );
Review Comment:
...and I do agree.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878007523
##########
maven-scm-providers/maven-scm-provider-hg/pom.xml:
##########
@@ -66,10 +66,17 @@
</plugins>
</build>
-
<profiles>
<profile>
<id>tck-hg</id>
+ <activation>
+ <property>
+ <name>run-its</name>
+ </property>
+ <file>
+ <exists>/usr/bin/hg</exists>
Review Comment:
Not portable
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r879133001
##########
maven-scm-providers/maven-scm-provider-local/src/main/java/org/apache/maven/scm/provider/local/command/update/LocalUpdateCommand.java:
##########
@@ -190,7 +190,7 @@ private List<ScmFile> update( File source, File baseDestination, List<File> file
int chop = baseDestination.getAbsolutePath().length();
- String fileName = "/" + destinationFile.getAbsolutePath().substring( chop + 1 );
+ String fileName = destinationFile.getAbsolutePath().substring( chop + 1 );
Review Comment:
And yes, this indeed a test that would probably fail on Windows too.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r879111927
##########
maven-scm-providers/maven-scm-provider-hg/src/test/java/org/apache/maven/scm/provider/hg/HgRepoUtils.java:
##########
@@ -35,9 +35,11 @@
*
* @author <a href="mailto:thurner.rupert@ymono.net">thurner rupert</a>
*/
-public class HgRepoUtils
+public abstract class HgRepoUtils
Review Comment:
This failed in the previous testing framework. It no longer does so I have remove this change.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878926485
##########
maven-scm-test/src/main/java/org/apache/maven/scm/tck/command/changelog/ChangeLogCommandTckTest.java:
##########
@@ -94,7 +94,6 @@ public void testChangeLogCommand()
ChangeSet changeset = thirdResult.getChangeLog().getChangeSets().get( 0 );
assertTrue( changeset.getDate().after( timeBeforeSecond ) );
-
- assertEquals( COMMIT_MSG, changeset.getComment() );
+ assertTrue( changeset.getComment().startsWith( COMMIT_MSG ) ); // Sometimes there is a newline appended
Review Comment:
Which? I would rather strip trailing LS and then go through `assertEquals()`
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878926275
##########
maven-scm-providers/maven-scm-provider-hg/src/test/java/org/apache/maven/scm/provider/hg/HgRepoUtils.java:
##########
@@ -35,9 +35,11 @@
*
* @author <a href="mailto:thurner.rupert@ymono.net">thurner rupert</a>
*/
-public class HgRepoUtils
+public abstract class HgRepoUtils
Review Comment:
Why? This class even extends `ScmTestCase` for whatsoever reason and has no descendants.
--
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 pull request #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1132696065
Apparently the tests now pass (on Ubuntu) but because not all Github build environments/images have all the tools needed they now fail on (at least) the macOS image.
`Caused by: java.io.IOException: Cannot run program "hg" (in directory ...): error=2, No such file or directory`
Looking at https://github.com/apache/maven-gh-actions-shared/blob/v2/.github/workflows/maven-verify.yml I see that it simply uses the macOS-latest image provided by Github.
Please advise on how you would like to proceed.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1133955767
@nielsbasjes Please rebase
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1132895850
@cstamas is now working on replacing Plexus. Does this intersect?
--
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 pull request #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1132745021
> assumeTrue
Ah yes, interesting.
In the mean time I found an alternative way: Activate the tck-... profiles on a run-its property (not profile) in combination with the existence of the needed tool.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1133977138
* `GitListCommandTckTest` hasn't obviously not being implemented...
* Looking at `GitInfoCommandTckTest` it does not seem seem to be a TCK test, but merely a unit test. It does not have the same structure as TCK tests.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r879770206
##########
maven-scm-providers/maven-scm-provider-hg/src/main/java/org/apache/maven/scm/provider/hg/command/changelog/HgChangeLogConsumer.java:
##########
@@ -148,7 +148,6 @@ else if ( line.startsWith( MESSAGE_TOKEN ) )
{
StringBuilder comment = new StringBuilder( currentChange.getComment() );
comment.append( line );
- comment.append( '\n' );
Review Comment:
WTF? Who had this idea to modify the original comment?
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r879140561
##########
maven-scm-test/src/main/java/org/apache/maven/scm/ScmTckTestCase.java:
##########
@@ -112,14 +114,19 @@ protected List<String> getScmFileNames()
public abstract void initRepo()
throws Exception;
+ private static final Logger LOG = LoggerFactory.getLogger( ScmTckTestCase.class );
+
@Before
public void checkScmPresence()
{
String scmProviderCommand = getScmProviderCommand();
if ( scmProviderCommand != null )
{
+ LOG.warn( "Checking for required SCM command \"{}\" --> {}",
+ scmProviderCommand, ScmTestCase.isSystemCmd( scmProviderCommand ) );
assumeTrue( "Skipping tests because the required command " + scmProviderCommand + " is not available.",
ScmTestCase.isSystemCmd( scmProviderCommand ) );
+ LOG.warn( "Required SCM command \"{}\" exists.", scmProviderCommand );
Review Comment:
It works now, on MacOS on Github I see:
```
2022-05-23T08:00:42.9665730Z [INFO]
2022-05-23T08:00:42.9767310Z [INFO] -------------------------------------------------------
2022-05-23T08:00:42.9868750Z [INFO] T E S T S
2022-05-23T08:00:42.9969860Z [INFO] -------------------------------------------------------
2022-05-23T08:00:43.2415810Z [INFO] Running org.apache.maven.scm.provider.hg.repository.HgScmProviderRepositoryTest
2022-05-23T08:00:43.3204390Z [INFO] Tests run: 6, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.077 s - in org.apache.maven.scm.provider.hg.repository.HgScmProviderRepositoryTest
2022-05-23T08:00:43.3223180Z [INFO] Running org.apache.maven.scm.provider.hg.HgUtilsTest
2022-05-23T08:00:43.3792390Z [INFO] Tests run: 2, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.056 s - in org.apache.maven.scm.provider.hg.HgUtilsTest
2022-05-23T08:00:43.3820030Z [INFO] Running org.apache.maven.scm.provider.hg.command.update.HgUpdateCommandTckTest
2022-05-23T08:00:43.4054560Z [WARNING] Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0.024 s - in org.apache.maven.scm.provider.hg.command.update.HgUpdateCommandTckTest
2022-05-23T08:00:43.4057710Z [INFO] Running org.apache.maven.scm.provider.hg.command.changelog.HgChangeLogCommandTckTest
2022-05-23T08:00:43.4130830Z [WARNING] Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0.006 s - in org.apache.maven.scm.provider.hg.command.changelog.HgChangeLogCommandTckTest
2022-05-23T08:00:43.4138580Z [INFO] Running org.apache.maven.scm.provider.hg.command.diff.HgDiffCommandTckTest
2022-05-23T08:00:43.4208220Z [WARNING] Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0.007 s - in org.apache.maven.scm.provider.hg.command.diff.HgDiffCommandTckTest
2022-05-23T08:00:43.4224000Z [INFO] Running org.apache.maven.scm.provider.hg.command.status.HgStatusCommandTckTest
2022-05-23T08:00:43.4280470Z [WARNING] Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0.006 s - in org.apache.maven.scm.provider.hg.command.status.HgStatusCommandTckTest
2022-05-23T08:00:43.4287380Z [INFO] Running org.apache.maven.scm.provider.hg.command.checkout.HgCheckOutCommandTckTest
2022-05-23T08:00:43.4345300Z [WARNING] Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0.005 s - in org.apache.maven.scm.provider.hg.command.checkout.HgCheckOutCommandTckTest
2022-05-23T08:00:43.4363420Z [INFO] Running org.apache.maven.scm.provider.hg.command.blame.HgBlameCommandTckTest
2022-05-23T08:00:43.4419300Z [WARNING] Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0.006 s - in org.apache.maven.scm.provider.hg.command.blame.HgBlameCommandTckTest
2022-05-23T08:00:43.4468160Z [INFO] Running org.apache.maven.scm.provider.hg.command.checkin.HgCheckInCommandTckTest
2022-05-23T08:00:43.4567260Z [WARNING] Tests run: 2, Failures: 0, Errors: 0, Skipped: 2, Time elapsed: 0.013 s - in org.apache.maven.scm.provider.hg.command.checkin.HgCheckInCommandTckTest
2022-05-23T08:00:43.4574170Z [INFO] Running org.apache.maven.scm.provider.hg.command.tag.HgTagCommandTckTest
2022-05-23T08:00:43.4628770Z [WARNING] Tests run: 1, Failures: 0, Errors: 0, Skipped: 1, Time elapsed: 0.005 s - in org.apache.maven.scm.provider.hg.command.tag.HgTagCommandTckTest
2022-05-23T08:00:44.6834330Z [INFO]
2022-05-23T08:00:44.6834780Z [INFO] Results:
2022-05-23T08:00:44.6837080Z [INFO]
2022-05-23T08:00:44.6837810Z [WARNING] Tests run: 17, Failures: 0, Errors: 0, Skipped: 9
```
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878032009
##########
maven-scm-providers/maven-scm-provider-hg/pom.xml:
##########
@@ -66,10 +66,17 @@
</plugins>
</build>
-
<profiles>
<profile>
<id>tck-hg</id>
+ <activation>
+ <property>
+ <name>run-its</name>
+ </property>
+ <file>
+ <exists>/usr/bin/hg</exists>
Review Comment:
I agree. I'll refactor using the assumeTrue you mentioned.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878925946
##########
maven-scm-test/src/main/java/org/apache/maven/scm/ScmTckTestCase.java:
##########
@@ -112,14 +114,19 @@ protected List<String> getScmFileNames()
public abstract void initRepo()
throws Exception;
+ private static final Logger LOG = LoggerFactory.getLogger( ScmTckTestCase.class );
+
@Before
public void checkScmPresence()
{
String scmProviderCommand = getScmProviderCommand();
if ( scmProviderCommand != null )
{
+ LOG.warn( "Checking for required SCM command \"{}\" --> {}",
+ scmProviderCommand, ScmTestCase.isSystemCmd( scmProviderCommand ) );
assumeTrue( "Skipping tests because the required command " + scmProviderCommand + " is not available.",
ScmTestCase.isSystemCmd( scmProviderCommand ) );
+ LOG.warn( "Required SCM command \"{}\" exists.", scmProviderCommand );
Review Comment:
I added this just now to figure out why it still fails on macOS.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878007766
##########
maven-scm-providers/maven-scm-providers-git/pom.xml:
##########
@@ -40,10 +40,18 @@
<module>maven-scm-provider-gitexe</module>
<module>maven-scm-provider-jgit</module>
</modules>
-
+
<profiles>
<profile>
<id>tck-git</id>
+ <activation>
+ <property>
+ <name>run-its</name>
+ </property>
+ <file>
+ <exists>/usr/bin/git</exists>
Review Comment:
Not portable
##########
maven-scm-providers/maven-scm-providers-svn/pom.xml:
##########
@@ -43,6 +43,14 @@
<profiles>
<profile>
<id>tck-svn</id>
+ <activation>
+ <property>
+ <name>run-its</name>
+ </property>
+ <file>
+ <exists>/usr/bin/svn</exists>
Review Comment:
Not portable
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r879774860
##########
maven-scm-providers/maven-scm-provider-hg/src/main/java/org/apache/maven/scm/provider/hg/command/changelog/HgChangeLogConsumer.java:
##########
@@ -148,7 +148,6 @@ else if ( line.startsWith( MESSAGE_TOKEN ) )
{
StringBuilder comment = new StringBuilder( currentChange.getComment() );
comment.append( line );
- comment.append( '\n' );
Review Comment:
Removing this seemed the cleanest solution to 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
[GitHub] [maven-scm] michael-o commented on pull request #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1132707261
There are tests which check wether `svn`, `git` as are installed via `assumeTrue`. Find those this you could apply to, I guess.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878925439
##########
maven-scm-test/src/main/java/org/apache/maven/scm/tck/command/changelog/ChangeLogCommandTckTest.java:
##########
@@ -94,7 +94,6 @@ public void testChangeLogCommand()
ChangeSet changeset = thirdResult.getChangeLog().getChangeSets().get( 0 );
assertTrue( changeset.getDate().after( timeBeforeSecond ) );
-
- assertEquals( COMMIT_MSG, changeset.getComment() );
+ assertTrue( changeset.getComment().startsWith( COMMIT_MSG ) ); // Sometimes there is a newline appended
Review Comment:
Under which circumstances did this occur to you?
--
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] asfgit closed pull request #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #140: [SCM-981] Add run-its profile and fix broken tests
URL: https://github.com/apache/maven-scm/pull/140
--
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 pull request #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1132890348
Seems like the whole test suite is running on very old code.
When I add `@RunWith(JUnit4.class)` to ScmTckTestCase and add `@Test` on all the methods this suddenly starts working ... at the cost of failures in the `PlexusTestCase` area.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r879117368
##########
maven-scm-test/src/main/java/org/apache/maven/scm/ScmTckTestCase.java:
##########
@@ -112,14 +114,19 @@ protected List<String> getScmFileNames()
public abstract void initRepo()
throws Exception;
+ private static final Logger LOG = LoggerFactory.getLogger( ScmTckTestCase.class );
+
@Before
public void checkScmPresence()
{
String scmProviderCommand = getScmProviderCommand();
if ( scmProviderCommand != null )
{
+ LOG.warn( "Checking for required SCM command \"{}\" --> {}",
+ scmProviderCommand, ScmTestCase.isSystemCmd( scmProviderCommand ) );
assumeTrue( "Skipping tests because the required command " + scmProviderCommand + " is not available.",
ScmTestCase.isSystemCmd( scmProviderCommand ) );
+ LOG.warn( "Required SCM command \"{}\" exists.", scmProviderCommand );
Review Comment:
Found it. Multiple `@Before` and no ordering guarantee among these.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878926063
##########
maven-scm-providers/maven-scm-provider-local/src/main/java/org/apache/maven/scm/provider/local/command/update/LocalUpdateCommand.java:
##########
@@ -190,7 +190,7 @@ private List<ScmFile> update( File source, File baseDestination, List<File> file
int chop = baseDestination.getAbsolutePath().length();
- String fileName = "/" + destinationFile.getAbsolutePath().substring( chop + 1 );
+ String fileName = destinationFile.getAbsolutePath().substring( chop + 1 );
Review Comment:
I have Ubuntu and it fails because the assertion expects it without a slash.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878926224
##########
maven-scm-test/src/main/java/org/apache/maven/scm/tck/command/changelog/ChangeLogCommandTckTest.java:
##########
@@ -94,7 +94,6 @@ public void testChangeLogCommand()
ChangeSet changeset = thirdResult.getChangeLog().getChangeSets().get( 0 );
assertTrue( changeset.getDate().after( timeBeforeSecond ) );
-
- assertEquals( COMMIT_MSG, changeset.getComment() );
+ assertTrue( changeset.getComment().startsWith( COMMIT_MSG ) ); // Sometimes there is a newline appended
Review Comment:
One of the SCMs returned it with a newline. Perhaps this should be fixed differently.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r879115362
##########
maven-scm-test/src/main/java/org/apache/maven/scm/ScmTckTestCase.java:
##########
@@ -112,14 +114,19 @@ protected List<String> getScmFileNames()
public abstract void initRepo()
throws Exception;
+ private static final Logger LOG = LoggerFactory.getLogger( ScmTckTestCase.class );
+
@Before
public void checkScmPresence()
{
String scmProviderCommand = getScmProviderCommand();
if ( scmProviderCommand != null )
{
+ LOG.warn( "Checking for required SCM command \"{}\" --> {}",
+ scmProviderCommand, ScmTestCase.isSystemCmd( scmProviderCommand ) );
assumeTrue( "Skipping tests because the required command " + scmProviderCommand + " is not available.",
ScmTestCase.isSystemCmd( scmProviderCommand ) );
+ LOG.warn( "Required SCM command \"{}\" exists.", scmProviderCommand );
Review Comment:
I have just now uninstalled hg from my local system and I'm now able to locally reproduce the problem.
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878925192
##########
maven-scm-providers/maven-scm-provider-local/src/main/java/org/apache/maven/scm/provider/local/command/update/LocalUpdateCommand.java:
##########
@@ -190,7 +190,7 @@ private List<ScmFile> update( File source, File baseDestination, List<File> file
int chop = baseDestination.getAbsolutePath().length();
- String fileName = "/" + destinationFile.getAbsolutePath().substring( chop + 1 );
+ String fileName = destinationFile.getAbsolutePath().substring( chop + 1 );
Review Comment:
Interesting that blindly a slash was prepended. That was doomed to fail on Windows, no?
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1135081659
Working on this...
```
[INFO]
[ERROR] Failures:
[ERROR] HgDiffCommandTckTest.testDiffCommand:86->ScmTckTestCase.addToWorkingTree:218 Expected 1 file in the added files list [] expected:<1> but was:<0>
[ERROR] HgStatusCommandTckTest>StatusCommandTckTest.testStatusCommand:107->ScmTckTestCase.addToWorkingTree:218 Expected 1 file in the added files list [] expected:<1> but was:<0>
[ERROR] HgUpdateCommandTckTest>UpdateCommandTckTest.testUpdateCommand:119->ScmTckTestCase.addToWorkingTree:218 Expected 1 file in the added files list [] expected:<1> but was:<0>
[INFO]
[ERROR] Tests run: 17, Failures: 3, Errors: 0, Skipped: 0
```
with
```
$ hg --version
Mercurial Distributed SCM (version 6.1.1)
(see https://mercurial-scm.org for more information)
Copyright (C) 2005-2022 Olivia Mackall and others
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
```
on
```
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
mosipov@bsd1srv:/usr/home/mosipov/var/Projekte/maven-scm (SCM-981 =)
$ mvn -v
Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /usr/local/apache-maven-3.6.3
Java version: 1.8.0_332, vendor: OpenJDK BSD Porting Team, runtime: /usr/local/openjdk8/jre
Default locale: de_DE, platform encoding: UTF-8
OS name: "freebsd", version: "12.3-stable", arch: "amd64", family: "unix"
```
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1132755099
Please see: https://issues.apache.org/jira/browse/SCM-939
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r879114718
##########
maven-scm-test/src/main/java/org/apache/maven/scm/tck/command/changelog/ChangeLogCommandTckTest.java:
##########
@@ -94,7 +94,6 @@ public void testChangeLogCommand()
ChangeSet changeset = thirdResult.getChangeLog().getChangeSets().get( 0 );
assertTrue( changeset.getDate().after( timeBeforeSecond ) );
-
- assertEquals( COMMIT_MSG, changeset.getComment() );
+ assertTrue( changeset.getComment().startsWith( COMMIT_MSG ) ); // Sometimes there is a newline appended
Review Comment:
Turns out that the HG code ... appends a newline. Fixed
--
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 pull request #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1134335226
Now all tests are run on all platforms where the needed tooling has been installed.
I put up this followup issue to improve the build environment to go towards all tests being run on all systems always.
https://issues.apache.org/jira/browse/SCM-988
--
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 pull request #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
nielsbasjes commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1133973802
I have
- Fixed a few broken tests.
- Added a tck-hg and tck-local for consistency
- Added a run-its profile that runs all.
- Added the method ScmTckTestCase::getScmProviderCommand which either returns null (i.e. no command needs to exist) or the name of the command that must exist.
This has been overridden in all `Hg...TckTest` , `SvnExe...TckTest` and `GitExe...TckTest` classes (except the two specials mentioned above). This method is checked before each test and ignores tests for which the required command is missing.
Classes with things I consider "unexpected" and have not changed:
- GitListCommandTckTest is abstract and not actually implemented anywhere.
- GitInfoCommandTckTest extends ScmTestCase instead of ScmTckTestCase
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on code in PR #140:
URL: https://github.com/apache/maven-scm/pull/140#discussion_r878925721
##########
maven-scm-test/src/main/java/org/apache/maven/scm/ScmTckTestCase.java:
##########
@@ -112,14 +114,19 @@ protected List<String> getScmFileNames()
public abstract void initRepo()
throws Exception;
+ private static final Logger LOG = LoggerFactory.getLogger( ScmTckTestCase.class );
+
@Before
public void checkScmPresence()
{
String scmProviderCommand = getScmProviderCommand();
if ( scmProviderCommand != null )
{
+ LOG.warn( "Checking for required SCM command \"{}\" --> {}",
+ scmProviderCommand, ScmTestCase.isSystemCmd( scmProviderCommand ) );
assumeTrue( "Skipping tests because the required command " + scmProviderCommand + " is not available.",
ScmTestCase.isSystemCmd( scmProviderCommand ) );
+ LOG.warn( "Required SCM command \"{}\" exists.", scmProviderCommand );
Review Comment:
Why warn? Is this intermediate?
--
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 #140: [SCM-981] Add run-its profile and fix broken tests
Posted by GitBox <gi...@apache.org>.
michael-o commented on PR #140:
URL: https://github.com/apache/maven-scm/pull/140#issuecomment-1133981667
You can rename: GitInfoCommandTckTest
--
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