Test tasks that only has been modified or added

[chmouel]

Changes

We are nnow only doing integration tests on the tasks that has been modified or added. This will improve the speed of each PR iteration not testing everything while the PR modify only some part of it.

/cc @sthaha
if you know if there is any other git trickery i can use to detect
file changes between the 'main' branch and the PR

/cc @bobcatfish
cf #373 discussion

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes docs (if user facing)
• Commit messages follow commit message best practices
• Yaml file complies with yamllint rules.

See the contribution guide
for more details.

[tekton-robot]

@chmouel: GitHub didn't allow me to request PR reviews from the following users: you, there, is, other, detect, -, to, git, trickery, i, can, use, if, know.

Note that only tektoncd members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Changes

Detecting the tasks that only has been modified or added and test only those in
PR. This will improve the speed of each PR iteration.

/hold

/cc @sthaha - if you know if there is any other git trickery i can use to detect
file changes between the 'main' branch and the PR

/cc @bobcatfish cf #373 discussion

Submitter Checklist

These are the criteria that every PR should meet, please check them off as you
review them:

• Includes docs (if user facing)
• Commit messages follow commit message best practices
• Yaml file complies with yamllint rules.

See the contribution guide
for more details.

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.

[vdemeester]

/cc @sbwsg @afrittoli

[vdemeester]

/lgtm
/meow
😻

[tekton-robot]

@vdemeester:

In response to this:

/lgtm
/meow
😻

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.

[chmouel]

Seems to work okay, it only detect the change in curl and wget :

+ all_tests='curl/tests
wget/tests'
...
+ test_task_creation curl/tests wget/tests

(i had another test that test as well new addition)

I am not detecting the changes for test_yaml_creation, I will add a further PR to add it but it's not such a big problem currently since those tests are pretty quick to execute compared to the integration tests

I think we would probably want some full integration weekly test run still, we may have set this up from the plumbing repository.

/hold cancel

[afrittoli]

While I understand the reason behind this, I fear this may come back and bite us, at least until we only have one repo for catalog. Many tasks will not change a lot over time, so they won't be tested for potentially long time - when the time comes to actually change something they might be not usable anymore.

So, while I agree this is good to have, I think at the same time we need to setup some way to run all the tests, either on demand, or periodically. Perhaps we should have an "info only" catalog test running on pipeline changes, or one that could be triggered on demand.

[chmouel]

So, while I agree this is good to have, I think at the same time we need to setup some way to run all the tests, either on demand, or periodically. Perhaps we should have an "info only" catalog test running on pipeline changes, or one that could be triggered on demand.

Yes agreed this is what i was mentioning in my comment here we would need to have some weekly periodic jobs for this...

I am not so sure if we can tell to prow something like this

/test integration-test eveyrthing

and have the argument everything get passed down to the e2e script. If that's possible we could allow contributors/reviewers to launch a full test when they think this should be appropriate.

I am not so sure I understand what do you mean by "info only"

[chmouel]

Further note, the integration tests before this PR is taking 20mn to run, if we are adding pipeline supported version matrix this time would exponentially grow pretty big

[bobcatfish]

Further note, the integration tests before this PR is taking 20mn to run, if we are adding pipeline supported version matrix this time would exponentially grow pretty big

only if they run one at a time :D

[bobcatfish]

i wonder if doing this will prevent us from catching issues - for example if we bump the version of tekton pipelines the tests are using, but we don't run any of the tests, we wont catch any problems 🤔

maybe we can add a nightly run of the tests to make up for it?

[chmouel]

I have added cc4b6f0 so we can define the variable TEST_RUN_ALL_TESTS to force tests everything....

Now to figure out on how to add a nightly catalog job in openshift/plumbing

[chmouel]

/hold

[popcor255]

All tests are isolated in the given namespace. They are created at the begging of tests and cleaned up at the end of tests. Since, namespace are fairly predictable "local tns="${testname}-${version}". Maybe we should clean up all the resources in the namespace and not the namespace itself when the test ends. We should maybe have a task at the begging of the test suite to clean up old namespaces. At which point we only run tests if the namespace does not exist.
line 107 (e2e-common.sh)

local tns="${testname}-${version}"

I am spit balling here, tbh. Just trying to give a suggesiton.

[tekton-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: 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:

• OWNERS [vdemeester]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[chmouel]

/hold cancel

this was tested as working, i have added previously a commit to this pr which had a change in the task/trigger-jenkins-job and it properly tested only that task

@vdemeester we are good to update the periodic jobs too.

[piyush-garg]

lgtm

[piyush-garg]

/lgtm