Add head name property in MercurialSCM #277
Conversation
There was a problem hiding this comment.
Feature does not require specific tests
Sure it does.
I guess the description is referring to https://github.com/jenkinsci/workflow-multibranch-plugin/blob/28fbf74dfcaeccbee5f7fbb1ae7d0d3c95cd932d/src/main/java/org/jenkinsci/plugins/workflow/multibranch/ResolveScmStep.java#L282 though it is not clear.
|
@long76 thanks for the pull request. I think that it would be valuable to add a test of the new methods that have been added so that future changes do not break the new methods. The Mercurial plugin is currently up for adoption. That means there is currently no active maintainer for the plugin. Since you are using the plugin, are you willing to adopt the plugin and become the maintainer of the plugin? |
thanks but no) we migrate to git but this functionality would be useful. i make our local version this plugin and deploy them. so what happens first our migration or plugin has maintainer) |
yes you are right. that's what i meant. |
|
@jglick @MarkEWaite add tests |
| MercurialSCM mercurialSCM = new MercurialSCMBuilder(entry.getKey(), entry.getValue(), "", "") | ||
| .build(); | ||
| assertEquals(MercurialSCM.RevisionType.CHANGESET, mercurialSCM.getRevisionType()); | ||
| assertEquals(entry.getValue().getHead().getName(), mercurialSCM.getHeadName()); |
There was a problem hiding this comment.
This test is just asserting that a newly introduced method has an expected value. Fine so far as it goes, but does not demonstrate that the purported purpose of the change (to fix some behavior of resolveScm) is actually accomplished by doing that. Is some piece of code using reflection to look for a JavaBeans property named headName? (I certainly hope not!)
There was a problem hiding this comment.
no reflection here of course) just check all existing heads) resolveScm works correct just it return MercurialSCM where you can get only hash of commit. in fact i dont know which test need for it but my tests confirm that headName correct. maybe need other name of variable) but it's not breaking change like change value of revision from hash to branch name
If use
resolveScmin usual pipeline you cannot get branch name -getRevisionreturn hash althoughtresolveScmprint and branch name too.Testing done
Submitter checklist