Add e2e tests using gRPC and REST #418

sayan-biswas · 2023-03-28T12:34:33Z

Changes

Add e2e test for ListRecords, ListResults, GetRecord, GetResult, DeleteRecord, DeleteResult, Authorization, Authentication and Impersonation using both gRPC and REST.

Submitter Checklist

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

Has Docs included if any changes are user facing
Has Tests included if any functionality added or changed
Tested your changes locally (if this is a code change)
Follows the commit message standard
Meets the Tekton contributor standards (including functionality, content, code)
Has a kind label. You can add a comment on this PR that contains /kind <type>. Valid types are bug, cleanup, design, documentation, feature, flake, misc, question, tep
Release notes block below has been updated with any user-facing changes (API changes, bug fixes, changes requiring upgrade notices or deprecation warnings)
Release notes contain the string "action required" if the change requires additional action from users switching to the new release

Release Notes

NONE

tekton-robot · 2023-03-28T12:34:36Z

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

sayan-biswas · 2023-04-03T20:51:39Z

/kind misc

sayan-biswas · 2023-04-03T20:51:58Z

/test all

sayan-biswas · 2023-04-03T21:03:27Z

/test pull-tekton-results-build-tests

sayan-biswas · 2023-04-05T07:33:05Z

/assign @adambkaplan

prietyc123 · 2023-04-11T11:33:47Z

test/e2e/e2e_test.go

 			if err != nil {
-				return false, nil
+				t.Fatalf("Error getting TaskRun: %v", err)


Shouldn't we add return false, err here. Also line https://github.com/tektoncd/results/pull/418/files#diff-d5d12b6ea07b0a78c9ad1987e1486c565ecf16839f77a93e91b09475cf6c0e99R131 to be executed we should throw err I think.

This is now running inside a test function that has no return. t.Fatalf is the right thing to do.

This is the return statement of the callback function to PollImmediate. I guess calling t.Fatal() here would mark the test as failed before even waiting for the timeout.

prietyc123 · 2023-04-11T11:39:17Z

test/e2e/e2e_test.go

 			if err != nil {
-				t.Logf("Get: %v", err)
-				return false, nil
+				t.Fatalf("Error getting PipelineRun: %v", err)


Same add return false, err

Ditto from above - we can't return any value here, and t.Fatalf stops the test.

This is the return statement of the callback function to PollImmediate. I guess calling t.Fatal() here would mark the test as failed before even waiting for the timeout.

test/e2e/e2e_test.go

config/base/default-clusterroles.yaml

test/e2e/client/grpc.go

test/e2e/kustomize/api-server.yaml

adambkaplan · 2023-04-11T19:43:41Z

test/e2e/e2e.sh

@@ -40,7 +40,7 @@ main() {

    # Build static binaries; otherwise go test complains.
    export CGO_ENABLED=0
-    go test -v -count=1 --tags=e2e ${REPO}/test/e2e/...
+    go test -v -count=1 --tags=e2e ${REPO}/test/e2e/e2e_test.go


Any particular reason why we should drive the test from the one file? This could lead to issues in the future if we define suites in different e2e test files.

I have changed the behaviour to include all package in e2e directory.

Initially this was done because the test was failing as the client directory under e2e doesn't have any _test files. Now I have just excluded the client directory from go test, while still building the files in the client directory.

adambkaplan · 2023-04-11T20:56:00Z

test/e2e/e2e_test.go

 			if err != nil {
-				return false, nil
+				t.Fatalf("Error getting TaskRun: %v", err)


This is now running inside a test function that has no return. t.Fatalf is the right thing to do.

adambkaplan · 2023-04-11T20:56:37Z

test/e2e/e2e_test.go

 			if err != nil {
-				t.Logf("Get: %v", err)
-				return false, nil
+				t.Fatalf("Error getting PipelineRun: %v", err)


Ditto from above - we can't return any value here, and t.Fatalf stops the test.

adambkaplan · 2023-04-11T20:59:33Z

/approve

Marking conditional approval so other contributors can add lgtm. The impersonation RBAC needs to be moved to our test overlay/kustomization, and I have a question about the tls hostname override. Otherwise looks good.

tekton-robot · 2023-04-11T20:59:37Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adambkaplan

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~~ [adambkaplan]

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

Add e2e test for ListRecords, ListResults, GetRecord, GetResult, DeleteRecord, DeleteResult, Authorization, Authentication and Impersonation using both gRPC and REST.

adambkaplan · 2023-04-13T23:00:47Z

@tektoncd/results-collaborators this PR is ready for an LGTM review.

adambkaplan · 2023-04-13T23:01:32Z

cc @tektoncd/results-maintainers

sayan-biswas · 2023-04-15T02:49:18Z

@khrm @enarha This should be in the release, as features from this release and previous release are cover in these e2e tests.

alan-ghelardi · 2023-04-16T00:52:05Z

/lgtm

tekton-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Mar 28, 2023

tekton-robot requested review from alan-ghelardi and dibyom March 28, 2023 12:34

tekton-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Mar 28, 2023

sayan-biswas force-pushed the e2e-rest branch from 8409fa5 to f04afe3 Compare March 29, 2023 23:52

tekton-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Mar 29, 2023

sayan-biswas force-pushed the e2e-rest branch from f04afe3 to b09a328 Compare April 3, 2023 20:39

tekton-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 3, 2023

sayan-biswas changed the title ~~e2e tests for REST~~ Add e2e tests using gRPC and REST Apr 3, 2023

tekton-robot added release-note-none Denotes a PR that doesnt merit a release note. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 3, 2023

sayan-biswas force-pushed the e2e-rest branch from b09a328 to ec00145 Compare April 3, 2023 20:49

tekton-robot added the kind/misc Categorizes issue or PR as a miscellaneuous one. label Apr 3, 2023

sayan-biswas marked this pull request as ready for review April 3, 2023 21:09

tekton-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 3, 2023

sayan-biswas force-pushed the e2e-rest branch from ec00145 to 28f5339 Compare April 3, 2023 21:15

tekton-robot assigned adambkaplan Apr 5, 2023

sayan-biswas force-pushed the e2e-rest branch 2 times, most recently from 4dd599a to 09f6bb9 Compare April 6, 2023 18:50

prietyc123 suggested changes Apr 11, 2023

View reviewed changes

test/e2e/e2e_test.go Show resolved Hide resolved

test/e2e/e2e_test.go Show resolved Hide resolved

adambkaplan reviewed Apr 11, 2023

View reviewed changes

tekton-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 11, 2023

sayan-biswas force-pushed the e2e-rest branch from 09f6bb9 to bf1821a Compare April 13, 2023 17:15

Test: Add e2e tests using gRPC and REST

b390819

Add e2e test for ListRecords, ListResults, GetRecord, GetResult, DeleteRecord, DeleteResult, Authorization, Authentication and Impersonation using both gRPC and REST.

sayan-biswas force-pushed the e2e-rest branch from bf1821a to b390819 Compare April 13, 2023 17:57

tekton-robot assigned alan-ghelardi Apr 16, 2023

tekton-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 16, 2023

tekton-robot merged commit b21a9a3 into tektoncd:main Apr 16, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add e2e tests using gRPC and REST #418

Add e2e tests using gRPC and REST #418

sayan-biswas commented Mar 28, 2023 •

edited by adambkaplan

Loading

tekton-robot commented Mar 28, 2023

sayan-biswas commented Apr 3, 2023

sayan-biswas commented Apr 3, 2023

sayan-biswas commented Apr 3, 2023

sayan-biswas commented Apr 5, 2023

prietyc123 Apr 11, 2023

adambkaplan Apr 11, 2023

sayan-biswas Apr 12, 2023

prietyc123 Apr 11, 2023

adambkaplan Apr 11, 2023

sayan-biswas Apr 12, 2023

adambkaplan Apr 11, 2023

sayan-biswas Apr 13, 2023

adambkaplan Apr 11, 2023

adambkaplan Apr 11, 2023

adambkaplan commented Apr 11, 2023

tekton-robot commented Apr 11, 2023

adambkaplan commented Apr 13, 2023

adambkaplan commented Apr 13, 2023

sayan-biswas commented Apr 15, 2023

alan-ghelardi commented Apr 16, 2023

Add e2e tests using gRPC and REST #418

Add e2e tests using gRPC and REST #418

Conversation

sayan-biswas commented Mar 28, 2023 • edited by adambkaplan Loading

Changes

Submitter Checklist

Release Notes

tekton-robot commented Mar 28, 2023

sayan-biswas commented Apr 3, 2023

sayan-biswas commented Apr 3, 2023

sayan-biswas commented Apr 3, 2023

sayan-biswas commented Apr 5, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adambkaplan commented Apr 11, 2023

tekton-robot commented Apr 11, 2023

adambkaplan commented Apr 13, 2023

adambkaplan commented Apr 13, 2023

sayan-biswas commented Apr 15, 2023

alan-ghelardi commented Apr 16, 2023

sayan-biswas commented Mar 28, 2023 •

edited by adambkaplan

Loading