Refactor integration tests #5572

aledbf · 2021-09-07T12:24:07Z

Description

Refactor integration tests using sigs.k8s.io/e2e-framework to simplify the setup and definition and cleanup of the test cases.

Related Issue(s)

N/A

How to test

cd test && go test -v ./...

or cd test && go test -v ./... -args -namespace <some namespace> to use a different namespace from the current one in the kubeconfig context

Release Notes

Refactor integration tests using [sigs.k8s.io/e2e-framework](https://sigs.k8s.io/e2e-framework)

TODO:

flag to filter tests to run
~~timeouts per test?~~
~~multiple packages~~
~~add checks before running the tests (check gitpod pods are running)~~
~~improve cleanup (closer)~~

codecov · 2021-09-07T13:05:36Z

Codecov Report

Merging #5572 (a862413) into main (da1919f) will decrease coverage by 0.53%.
The diff coverage is 100.00%.

❗ Current head a862413 differs from pull request most recent head f165e30. Consider uploading reports for the commit f165e30 to get more accurate results

@@            Coverage Diff             @@
##             main    #5572      +/-   ##
==========================================
- Coverage   38.35%   37.81%   -0.54%     
==========================================
  Files          15       16       +1     
  Lines        3872     4152     +280     
==========================================
+ Hits         1485     1570      +85     
- Misses       2264     2455     +191     
- Partials      123      127       +4

Flag	Coverage Δ
components-local-app-app-linux	`?`
components-local-app-app-windows	`?`
components-supervisor-app	`37.81% <100.00%> (?)`
components-ws-manager-app	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
components/supervisor/pkg/supervisor/tasks.go	`57.43% <100.00%> (ø)`
...-manager/pkg/manager/internal/workpool/workpool.go
components/ws-manager/pkg/manager/create.go
components/ws-manager/pkg/manager/manager_ee.go
components/ws-manager/pkg/clock/clock.go
...omponents/ws-manager/pkg/manager/pod_controller.go
components/ws-manager/pkg/manager/monitor.go
...s/ws-manager/pkg/manager/internal/grpcpool/pool.go
components/ws-manager/pkg/manager/metrics.go
components/ws-manager/pkg/manager/status.go
... and 21 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da1919f...f165e30. Read the comment docs.

aledbf · 2021-09-08T00:03:36Z

/werft run

👍 started the job as gitpod-build-aledbf-integration.45

aledbf · 2021-09-08T00:13:05Z

/werft run

👍 started the job as gitpod-build-aledbf-integration.47

aledbf · 2021-09-08T10:15:22Z

/werft run

👍 started the job as gitpod-build-aledbf-integration.51

csweichel · 2021-09-09T10:05:16Z

👍 I like the way the e2e framework works, that it's just a thin layer on top of testing and doesn't shift to a different paradigm (think BDD with ginko)
👍 The removal of the integration test struct makes sense and makes the setup/teardown more explicit. That was a concept that didn't really work before
👍 Propagating state through context feels close to home
👎 All tests are now robed into the main package which is a bit messy. It'd be great if we could split that up again

Overall I think this approach makes sense. I'm looking forward to see how we can filter for the labels and features once that bit is merged in the e2e testing framework.

aledbf · 2021-09-09T11:52:27Z

All tests are now robed into the main package which is a bit messy. It'd be great if we could split that up again

Agree 💯

I'm looking forward to see how we can filter for the labels and features once that bit is merged in the e2e testing framework.

The WIP of such feature is here kubernetes-sigs/e2e-framework#48

csweichel · 2021-09-10T07:01:33Z

The WIP of such feature is here kubernetes-sigs/e2e-framework#48

Looks like we can already make use of the labels using --labels. I reckon in some future change we can use the changeset to determine the set of tests we need to execute, and filter them based on the labels. Also, we could add something like a smoketest label useful for testing the most basic functionality.

rl-gitpod

+1 this is a big improvement. 100% agree with Chris on the pros/cons.

Note that I've made changes to the tests here that will need to be merged once it is approved. More than happy to do that if it helps.

test/pkg/integration/context/context.go

test/pkg/integration/setup.go

rl-gitpod · 2021-09-17T05:15:21Z

test/pkg/integration/context/context.go

+
+type workspaceKey struct{}
+
+func GetComponentAPI(ctx context.Context) *integration.ComponentAPI {


/nit

Maybe 'MustGet*' variants to make the calling code a touch more explicit?

rl-gitpod · 2021-09-17T05:24:58Z

test/tests/components/content-service/content-service_test.go

+		},
+	}
+
+	uploadUrlRequest := features.New("UploadUrlRequest").


There is some variability in the feature naming throughout this PR and given they can be used to filter tests we should standardise on feature naming, i.e. camelcase (or not), spaces Y/N, should they always start with the package to allow grouping, etc.

Yes, that's one the missing definitions

test/tests/components/content-service/content-service_test.go

test/tests/components/database/db_test.go

test/tests/components/ws-manager/content_test.go

aledbf · 2021-09-17T09:50:11Z

Note that I've made changes to the tests here that will need to be merged once it is approved. More than happy to do that if it helps.

Yes, the idea is to merge any pending PR that changes integration tests before this one.

aledbf · 2021-09-29T21:20:05Z

/werft run

👍 started the job as gitpod-build-aledbf-integration.81

aledbf · 2021-09-30T11:34:50Z

/werft run

👍 started the job as gitpod-build-aledbf-integration.89

aledbf · 2021-09-30T13:36:11Z

/hold cancel

csweichel · 2021-09-30T13:37:28Z

/lgtm

roboquat · 2021-09-30T13:37:32Z

LGTM label has been added.

Git tree hash: 987aadbdffa8a7560ddc52ca8cd771669c2d853e

csweichel · 2021-09-30T13:39:10Z

/lgtm
/approve no-issue

roboquat · 2021-09-30T13:39:13Z

LGTM label has been added.

Git tree hash: 987aadbdffa8a7560ddc52ca8cd771669c2d853e

corneliusludmann · 2021-09-30T13:51:10Z

/approve no-issue

jankeromnes · 2021-09-30T14:31:13Z

Already green-lit and approved multiple times, and a quick glance revealed no obvious problems. Thanks! Rubber-stamping.

/approve no-issue

roboquat · 2021-09-30T14:31:22Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: corneliusludmann, csweichel, jankeromnes

Associated issue requirement bypassed by: corneliusludmann, csweichel, jankeromnes

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~~ [corneliusludmann,csweichel,jankeromnes]
~~components/blobserve/OWNERS~~ [csweichel]
~~components/gitpod-protocol/OWNERS~~ [jankeromnes]
~~components/proxy/OWNERS~~ [jankeromnes]
~~components/service-waiter/OWNERS~~ [jankeromnes]
~~components/supervisor/OWNERS~~ [corneliusludmann]
~~components/workspacekit/OWNERS~~ [csweichel]
~~components/ws-daemon/OWNERS~~ [csweichel]
~~components/ws-manager/OWNERS~~ [csweichel]
~~components/ws-manager-api/OWNERS~~ [csweichel]
~~components/ws-proxy/OWNERS~~ [csweichel]
~~dev/gpctl/OWNERS~~ [csweichel]

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

roboquat added do-not-merge/work-in-progress do-not-merge/release-note-label-needed labels Sep 7, 2021

roboquat requested review from AlexTugarev and rl-gitpod September 7, 2021 12:24

roboquat added the size/XXL label Sep 7, 2021

aledbf force-pushed the aledbf/integration branch from bcee404 to 0fb9658 Compare September 7, 2021 12:53

aledbf force-pushed the aledbf/integration branch 3 times, most recently from 3b25901 to 2c39d87 Compare September 7, 2021 18:30

roboquat added release-note and removed do-not-merge/release-note-label-needed labels Sep 7, 2021

aledbf force-pushed the aledbf/integration branch 3 times, most recently from 90b08c4 to 2361e28 Compare September 7, 2021 23:34

aledbf force-pushed the aledbf/integration branch from 2361e28 to 8a34e37 Compare September 8, 2021 00:08

aledbf force-pushed the aledbf/integration branch from 8a34e37 to a4d6855 Compare September 8, 2021 00:39

aledbf force-pushed the aledbf/integration branch 2 times, most recently from 23b3ee5 to 2531b8c Compare September 16, 2021 14:02

rl-gitpod reviewed Sep 17, 2021

View reviewed changes

geropl self-requested a review September 20, 2021 08:21

geropl mentioned this pull request Sep 20, 2021

[dev] Fix broken integration test execution on staging #5764

Closed

aledbf force-pushed the aledbf/integration branch from e46d51f to fbb46e2 Compare September 20, 2021 13:10

Update go dependencies

eb501e7

aledbf force-pushed the aledbf/integration branch from 4f55be2 to bd753e5 Compare September 29, 2021 12:53

aledbf force-pushed the aledbf/integration branch 3 times, most recently from 36d1a99 to bd753e5 Compare September 30, 2021 01:18

aledbf force-pushed the aledbf/integration branch from 94b4b9f to aef5bed Compare September 30, 2021 13:05

roboquat removed the do-not-merge/hold label Sep 30, 2021

roboquat assigned csweichel Sep 30, 2021

roboquat added the lgtm label Sep 30, 2021

aledbf added 4 commits September 30, 2021 10:38

Refactoring integration tests

11c86e2

Update test build script

3a7334c

Disable TestPrebuildWorkspaceTaskFail

fde3c9a

Fix random timeouts

f165e30

aledbf force-pushed the aledbf/integration branch from aef5bed to f165e30 Compare September 30, 2021 13:38

roboquat removed the lgtm label Sep 30, 2021

roboquat added the lgtm label Sep 30, 2021

roboquat added the approved label Sep 30, 2021

roboquat merged commit d862429 into main Sep 30, 2021

roboquat deleted the aledbf/integration branch September 30, 2021 14:31

rl-gitpod mentioned this pull request Sep 30, 2021

Add integration test for workspace backup #5970

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor integration tests #5572

Refactor integration tests #5572

aledbf commented Sep 7, 2021 •

edited

Loading

codecov bot commented Sep 7, 2021 •

edited

Loading

aledbf commented Sep 8, 2021 •

edited by werft-gitpod-dev-com bot

Loading

aledbf commented Sep 8, 2021 •

edited by werft-gitpod-dev-com bot

Loading

aledbf commented Sep 8, 2021 •

edited by werft-gitpod-dev-com bot

Loading

csweichel commented Sep 9, 2021 •

edited by werft-gitpod-dev-com bot

Loading

aledbf commented Sep 9, 2021

csweichel commented Sep 10, 2021

rl-gitpod left a comment

rl-gitpod Sep 17, 2021

rl-gitpod Sep 17, 2021

aledbf Sep 20, 2021

aledbf commented Sep 17, 2021

aledbf commented Sep 29, 2021 •

edited by werft-gitpod-dev-com bot

Loading

aledbf commented Sep 30, 2021 •

edited by werft-gitpod-dev-com bot

Loading

aledbf commented Sep 30, 2021

csweichel commented Sep 30, 2021

roboquat commented Sep 30, 2021

csweichel commented Sep 30, 2021

roboquat commented Sep 30, 2021

corneliusludmann commented Sep 30, 2021

jankeromnes commented Sep 30, 2021

roboquat commented Sep 30, 2021


		type workspaceKey struct{}

		func GetComponentAPI(ctx context.Context) *integration.ComponentAPI {

Refactor integration tests #5572

Refactor integration tests #5572

Conversation

aledbf commented Sep 7, 2021 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

codecov bot commented Sep 7, 2021 • edited Loading

Codecov Report

aledbf commented Sep 8, 2021 • edited by werft-gitpod-dev-com bot Loading

aledbf commented Sep 8, 2021 • edited by werft-gitpod-dev-com bot Loading

aledbf commented Sep 8, 2021 • edited by werft-gitpod-dev-com bot Loading

csweichel commented Sep 9, 2021 • edited by werft-gitpod-dev-com bot Loading

aledbf commented Sep 9, 2021

csweichel commented Sep 10, 2021

rl-gitpod left a comment

Choose a reason for hiding this comment

rl-gitpod Sep 17, 2021

Choose a reason for hiding this comment

rl-gitpod Sep 17, 2021

Choose a reason for hiding this comment

aledbf Sep 20, 2021

Choose a reason for hiding this comment

aledbf commented Sep 17, 2021

aledbf commented Sep 29, 2021 • edited by werft-gitpod-dev-com bot Loading

aledbf commented Sep 30, 2021 • edited by werft-gitpod-dev-com bot Loading

aledbf commented Sep 30, 2021

csweichel commented Sep 30, 2021

roboquat commented Sep 30, 2021

csweichel commented Sep 30, 2021

roboquat commented Sep 30, 2021

corneliusludmann commented Sep 30, 2021

jankeromnes commented Sep 30, 2021

roboquat commented Sep 30, 2021

aledbf commented Sep 7, 2021 •

edited

Loading

codecov bot commented Sep 7, 2021 •

edited

Loading

aledbf commented Sep 8, 2021 •

edited by werft-gitpod-dev-com bot

Loading

aledbf commented Sep 8, 2021 •

edited by werft-gitpod-dev-com bot

Loading

aledbf commented Sep 8, 2021 •

edited by werft-gitpod-dev-com bot

Loading

csweichel commented Sep 9, 2021 •

edited by werft-gitpod-dev-com bot

Loading

aledbf commented Sep 29, 2021 •

edited by werft-gitpod-dev-com bot

Loading

aledbf commented Sep 30, 2021 •

edited by werft-gitpod-dev-com bot

Loading