Skip some tests if KO_DOCKER_REPO is missing… #710
Conversation
googlebot
added
the
cla: yes
tekton-robot
added
approved
vdemeester
force-pushed
the
tests-skip-if
branch
2 times, most recently
from
978c144 to
7fa00ae
Compare
|
/test pull-tekton-pipeline-integration-tests |
|
I think I'd rather have tests fail loudly than silently pass if a prerequisite isn't provided. Ignoring failing tests like this can lead to surprising bugs that slip by tests. If there's some reason why it should be possible to run all e2e tests without setting In what scenarios should |
|
Discussed with @imjasonh on slack, let's go for a test flag to skip them instead of failing (and default to failing, so… no behaviour change on the CI) |
|
/hold |
tekton-robot
added
the
do-not-merge/hold
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: arilivigni, vdemeester 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 |
|
@arilivigni: changing LGTM is restricted to assignees, and only tektoncd/pipeline repo collaborators may be assigned issues. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
These tests are already skipped if KO_DOCKER_REPO is not set, why add another flag to skip them? |
|
@pivotal-nader-ziada the idea is to make it |
vdemeester
force-pushed
the
tests-skip-if
branch
from
7fa00ae to
edbae90
Compare
tekton-robot
added
size/M
… instead of "hard" failing, we could just skip it as it doesn't meet the requirements. Impacted tests : - TestHelmDeployPipelineRun - TestKanikoTaskRun To skip those tests instead of failing, you need to provide the following arguments to go test command : `-ldflags '-X github.com/tektoncd/pipeline/test.missingKoFatal=false'` Signed-off-by: Vincent Demeester <[email protected]>
vdemeester
force-pushed
the
tests-skip-if
branch
from
edbae90 to
e219cf7
Compare
|
/hold cancel |
tekton-robot
removed
the
do-not-merge/hold
|
/lgtm |
| return "", fmt.Errorf("KO_DOCKER_REPO env variable is required") | ||
| } | ||
| return fmt.Sprintf("%s/kanikotasktest", dockerRepo), nil | ||
| } |
There was a problem hiding this comment.
another beautiful file 💃
Changes
… instead of "hard" failing, we could just skip it as it doesn't meet
the requirements. Impacted tests :
To skip those tests instead of failing, you need to provide the following arguments to go test command :
-ldflags '-X github.com/tektoncd/pipeline/test.missingKoFatal=false'Signed-off-by: Vincent Demeester [email protected]
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
[ ] Includes docs (if user facing)See the contribution guide
for more details.