Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix testAvoidRedundantFetchWithHonorRefSpec assertions #926

Merged

Conversation

MarkEWaite
Copy link
Contributor

Fix testAvoidRedundantFetchWithHonorRefSpec assertions

The previous implementation of the test failed when the branch name 'master' is used because the Jenkins job directory name includes the branch name. This now asserts more precisely on the content of the FETCH_HEAD
file.

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that verify my changes
  • Unit tests pass locally with my changes
  • I have added documentation as necessary
  • No Javadoc warnings were introduced with my changes
  • No spotbugs warnings were introduced with my changes
  • I have tested my changes on multiple operating systems and CLI git versions
  • Any dependent changes have been merged and published in upstream modules (like git-client-plugin)

Types of changes

  • Dependency or infrastructure update

Further comments

As part of the exploration to find the problem, additional assertions have been added to confirm that the correct SHA1 is mentioned in the FETCH_HEAD file.

The previous test implementations would create a new CheckoutCommand with git.checkout() but then did nothing with it. It created the "foo" branch but did not checkout the "foo" branch. The commit that was described as "Commit in foo" was a commit to the master branch, not a commit to the foo branch.

The new code creates the branch and then uses the branch for new commits.

The previous implementation of the test would create a new CheckoutCommand
with git.checkout() but then did nothing with it.  It created the
"foo" branch but did not checkout the "foo" branch.  The commit that
was described as "Commit in foo" was a commit to the master branch,
not a commit to the foo branch.
The previous implementation of the test would create a new CheckoutCommand
with git.checkout() but then did nothing with it.  It created the
"foo" branch but did not checkout the "foo" branch.  The commit that
was described as "Commit in foo" was a commit to the master branch,
not a commit to the foo branch.
The previous implementation of the test would create a new CheckoutCommand
with git.checkout() but then did nothing with it.  It created the
"foo" branch but did not checkout the "foo" branch.  The commit that
was described as "Commit in foo" was a commit to the master branch,
not a commit to the foo branch.
The previous implementation of the test failed when the branch name
'master' is used because the Jenkins job directory name includes the
branch name.  This now asserts more precisely on the content of the
FETCH_HEAD file.

As part of the exploration to find the problem, additional assertions
have been added to confirm that the correct SHA1 is mentioned in the
FETCH_HEAD file.

Use correct branch in GitSCMTest.testAvoidRedundantFetchWithHonorRefSpec

The previous implementation of the test would create a new CheckoutCommand
with git.checkout() but then did nothing with it.  It created the
"foo" branch but did not checkout the "foo" branch.  The commit that
was described as "Commit in foo" was a commit to the master branch,
not a commit to the foo branch.
@MarkEWaite MarkEWaite requested a review from fcojfernandez July 2, 2020 11:49
final List<String> buildLog = build.getLog(50);
assertThat("master branch was fetched: " + buildLog, fetchHeadContents, not(containsString("branch 'master'")));
assertThat("foo branch was not fetched: " + buildLog, fetchHeadContents, containsString("branch 'foo'"));
assertThat("master branch SHA1 '" + commitFile1SHA1a + "' fetched " + buildLog, fetchHeadContents, not(containsString(commitFile1SHA1a)));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great assertion to exactly map the SHA1.

@rishabhBudhouliya
Copy link
Contributor

@MarkEWaite Thanks for fixing those tests, I was always a little confused with master and foo branch. Now I know.
LGTM

@MarkEWaite MarkEWaite merged commit bb7a3db into jenkinsci:master Jul 2, 2020
@MarkEWaite MarkEWaite deleted the use-correct-branches-in-GitSCMTest branch July 2, 2020 13:45
@MarkEWaite MarkEWaite added enhancement Improvement or new feature test and removed enhancement Improvement or new feature labels Jul 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants