Git pull on fetch fail plus tests #555
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes tektoncd#543 where the code source fails to be extracted from git, this happens when an old commit id is used that is not the HEAD of any branch.
googlebot
added
the
cla: yes
knative-prow-robot
added
the
size/L
knative-prow-robot
added
the
needs-ok-to-test
|
/ok-to-test |
knative-prow-robot
removed
the
needs-ok-to-test
nader-ziada
reviewed
Feb 27, 2019
There was a problem hiding this comment.
Thanks @dibbles for the PR, that should fix the issue, just had a couple of comments about the test
|
Additional commit added after review comments from @pivotal-nader-ziada |
|
looks good to me, thanks @dibbles /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: dibbles, pivotal-nader-ziada The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
knative-prow-robot
added
the
approved
|
@imjasonh @shashwathi might be a good idea to port a similar change to |
|
Excellent @dibbles !! Thanks so much for taking care of this :D (I'm gonna update the release notes to include the bug fix info) |
shashwathi
pushed a commit
to shashwathi/build
that referenced
this pull request
Mar 25, 2019
copying solution from tektoncd pipeline: tektoncd/pipeline#555 Credit to @dibbles on the original PR
knative-prow-robot
pushed a commit
to knative/build
that referenced
this pull request
Mar 26, 2019
* Add support for git commit sha copying solution from tektoncd pipeline: tektoncd/pipeline#555 Credit to @dibbles on the original PR * Add verification of default SA creation in e2e - Previous e2e test failed as namespace did not have default SA. Now the test setup verifies this exists
vdemeester
pushed a commit
to vdemeester/knative-build
that referenced
this pull request
Apr 3, 2019
* Add support for git commit sha copying solution from tektoncd pipeline: tektoncd/pipeline#555 Credit to @dibbles on the original PR * Add verification of default SA creation in e2e - Previous e2e test failed as namespace did not have default SA. Now the test setup verifies this exists
vdemeester
pushed a commit
to vdemeester/knative-build
that referenced
this pull request
Apr 3, 2019
* Add support for git commit sha copying solution from tektoncd pipeline: tektoncd/pipeline#555 Credit to @dibbles on the original PR * Add verification of default SA creation in e2e - Previous e2e test failed as namespace did not have default SA. Now the test setup verifies this exists
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #543 a bug where the code source fails to be extracted from git, this happens when an old commit id is used that is not the HEAD of any branch.
Changes
If the git fetch happens to error, a git pull is attempted. Whilst the run method has been modified to return an error, the error is only checked in the case of the git fetch, this was done to minimize the code change at this time.
Some tests have been added that use differing revisions (short commitID, long commitID, branch name etc...) and ensure pipelines run to completion. An additional test was added to ensure that if a revision is specified that is not valid, we receive the expected errors from git fetch and git pull.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
Release Notes
No user facing changes - bug fix - no doc changes needed
Additional Note - running tests locally required changing some timeouts. These changes have not been added with this initial commit as unknown whether this was specific to the environment in which I was running.