Fix running integration tests as part of build #12693

mads-hartmann · 2022-09-06T12:59:11Z

Description

This PR makes it possible to use with-integration-tests to run integration tests as part of the build job. This functionality has existed for a long time, but it was never updated to work with Harvester-based preview environments.

The old approach was to start a separate Werft job which used the eu.gcr.io/gitpod-core-dev/build/integration-tests:{{ .Annotations.version }} image in Werft. This meant it would run the compiled integration tests. However, this image doesn't have any of the utility scripts available we need to configure access to the the preview environment.

So I instead decided to delete the .werft/run-integration-tests.yaml job and instead introduce a small helper script -
test/run.sh - which uses go to run the tests. This is similar to the approach adopted by Workspace (.werft/workspace-run-integration-tests.sh) and IDE (.werft/ide-integration-tests-startup.yaml).

My intention is to use test/run.sh in both the Workspace and IDE integration test jobs too, so that we have a single way to run integration tests which is used in both Werft and Gitpod workspaces - but I'll look into this in a follow up PR.

This PR also adds a new checkbox for with-integration-tests to the PR template so spread awareness of the flag.

Related Issue(s)

Part of #12350

How to test

I ran the script manually from my Gitpod workspace

./test/run.sh
./test/run.sh ide
./test/run.sh webapp
./test/run.sh workspace

I ran it through Werft (see job)

werft job run github -a with-preview="true" -a with-integration-tests="ide"

Release Notes

NONE

Documentation

N/A

Werft options:

/werft with-preview
/werft with-integration-tests=all
Valid updates are all, workspace, webapp, ide

mads-hartmann · 2022-09-07T05:44:54Z

/werft run

👍 started the job as gitpod-build-mads-fix-with-integration-tests.16
(with .werft/ from main)

werft-gitpod-dev-com · 2022-09-07T14:18:04Z

started the job as gitpod-build-mads-fix-with-integration-tests.32 because the annotations in the pull request description changed
(with .werft/ from main)

werft-gitpod-dev-com · 2022-09-14T09:54:06Z

started the job as gitpod-build-mads-fix-with-integration-tests.39 because the annotations in the pull request description changed
(with .werft/ from main)

werft-gitpod-dev-com · 2022-09-14T09:54:08Z

started the job as gitpod-build-mads-fix-with-integration-tests.40 because the annotations in the pull request description changed
(with .werft/ from main)

mads-hartmann · 2022-09-14T11:34:54Z

@utam0k I'd like you to review this as you've been working on integration tests for workspace recently. Does the README capture how you have been working on integration tests? Are there any instructions or tips & tricks missing? ☺️
@mustard-mh I believe you're worked on integration tests for IDE. If so, I'd love your review on this PR too.
@geropl Wasn't sure who knew most about the previous setup but thought it might be you.

utam0k · 2022-09-15T08:06:02Z

@mads-hartmann Thanks for your great PR.
I don't think we can't get slack notifications when we do a werft run this way, right? I want to eventually run integration tests daily and receive notifications of them via slack.

test/run.sh

geropl

Thank you @mads-hartmann , tested and works! 🚀

mads-hartmann · 2022-09-15T09:14:02Z

I don't think we can't get slack notifications when we do a werft run this way, right? I want to eventually run integration tests daily and receive notifications of them via slack.

Yeah that's right, currently it won't send Slack notifications. After this PR is merged I'll look into seeing how we can have the IDE and Workspace integration tests jobs use this script.

There are two approaches (at least)

Have the IDE and Workspace integration test jobs use --with-integration-test=<...> when trigger the build job on their integration test branches.
Have the IDE and Workspace integration test jobs use run.sh too

Either way, we'd have to figure out exactly how to trigger the Slack notifications. But let's take that after this PR is merged 🧡

Co-authored-by: Gero Posmyk-Leinemann <[email protected]>

mads-hartmann · 2022-09-16T08:59:52Z

Rebase on main to remove conflict (an image version was bumped in run-integration-tests which this PR removes entirely)

liam-j-bennett · 2022-09-16T09:07:27Z

.werft/jobs/build/job-config.ts

+    switch (value) {
+        case null:
+        case undefined:
+            return "skip";


Why do we default to skip here, but for empty-string/unknown we default to all?

It's mainly about how we want to interpret how people use (or not use) the with-integration-tests job attribute, especially when used in GH comments/PR descriptions where you can use /werft run with-integration-tests or if you just set the attribute in your PR description (/werft with-integration-tests - in both cases without specifying a value - which case the value is the empty string.

In the CLI it's less common but you could do werft job run github -a with-integration-tests="".

So if it is null/undefined it means they didn't use the with-integration-tests attribute at all - if it's the empty string they did use it, but just didn't specify a value - I decided to interpret that as "they want to run all the tests"

Would it be useful to add this explanation as a comment in the code, or do you think it is overkill?

I added a log line of

werft.log(sliceID, "with-integration-tests was not set - will use 'skip'");

As I think that will

Capture the essence of the description above. Each of the three cases (not set, set to empty, set to unknown value) now have a log line that describe things a bit

This might be useful as it will help people know if they made a typo in the attribute, e.g. "with-integration-test" (missing s)

liam-j-bennett

🚀

iQQBot

Thanks! LGTM

iQQBot · 2022-09-16T15:01:00Z

For the IDE we currently have two completely separate sets (vscode, jetbrains) of integration tests that are not really connected to each other, it would be nice if they could be separated, but that can be something for later, thank you very much for your work! 🎉

roboquat added release-note-none do-not-merge/work-in-progress size/L labels Sep 6, 2022

mads-hartmann force-pushed the mads/fix-with-integration-tests branch from fd9729f to f2a2753 Compare September 7, 2022 10:51

mads-hartmann force-pushed the mads/fix-with-integration-tests branch from 21230d3 to c14dfdc Compare September 14, 2022 08:36

mads-hartmann changed the title ~~WIP: Fix running integration tests as part of build~~ Fix running integration tests as part of build Sep 14, 2022

mads-hartmann force-pushed the mads/fix-with-integration-tests branch from c14dfdc to 6acb87a Compare September 14, 2022 08:59

roboquat added size/XL and removed size/L labels Sep 14, 2022

mads-hartmann force-pushed the mads/fix-with-integration-tests branch 2 times, most recently from 4e05203 to 3044bdd Compare September 14, 2022 09:46

mads-hartmann force-pushed the mads/fix-with-integration-tests branch 3 times, most recently from ccc51b2 to f84e4bd Compare September 14, 2022 11:25

mads-hartmann marked this pull request as ready for review September 14, 2022 11:30

roboquat removed the do-not-merge/work-in-progress label Sep 14, 2022

mads-hartmann requested a review from a team September 14, 2022 11:31

github-actions bot added the team: devx label Sep 14, 2022

mads-hartmann requested a review from utam0k September 14, 2022 11:32

mads-hartmann added the do-not-merge/hold label Sep 14, 2022

mads-hartmann requested review from mustard-mh and geropl September 14, 2022 11:32

mustard-mh requested a review from iQQBot September 14, 2022 11:59

geropl reviewed Sep 15, 2022

View reviewed changes

test/run.sh Outdated Show resolved Hide resolved

geropl approved these changes Sep 15, 2022

View reviewed changes

mads-hartmann and others added 2 commits September 16, 2022 08:59

Fix ability to run integration tests in Werft

f52ce1e

Fix typo

77a6c85

Co-authored-by: Gero Posmyk-Leinemann <[email protected]>

mads-hartmann force-pushed the mads/fix-with-integration-tests branch from 0df8deb to 77a6c85 Compare September 16, 2022 08:59

liam-j-bennett reviewed Sep 16, 2022

View reviewed changes

liam-j-bennett approved these changes Sep 16, 2022

View reviewed changes

Add log when using 'skip'

a766c20

iQQBot approved these changes Sep 16, 2022

View reviewed changes

iQQBot removed the request for review from mustard-mh September 16, 2022 15:03

mads-hartmann removed the do-not-merge/hold label Sep 19, 2022

roboquat merged commit d9fe8fa into main Sep 19, 2022

roboquat deleted the mads/fix-with-integration-tests branch September 19, 2022 09:27

mads-hartmann mentioned this pull request Sep 22, 2022

[integration tests] provide a means to run tests from PR via opt-in, non-blocking mechanism #12350

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix running integration tests as part of build #12693

Fix running integration tests as part of build #12693

mads-hartmann commented Sep 6, 2022 •

edited

Loading

mads-hartmann commented Sep 7, 2022 •

edited by werft-gitpod-dev-com bot

Loading

werft-gitpod-dev-com bot commented Sep 7, 2022

werft-gitpod-dev-com bot commented Sep 14, 2022

werft-gitpod-dev-com bot commented Sep 14, 2022

mads-hartmann commented Sep 14, 2022 •

edited by werft-gitpod-dev-com bot

Loading

utam0k commented Sep 15, 2022 •

edited

Loading

geropl left a comment

mads-hartmann commented Sep 15, 2022 •

edited by werft-gitpod-dev-com bot

Loading

mads-hartmann commented Sep 16, 2022

liam-j-bennett Sep 16, 2022 •

edited

Loading

mads-hartmann Sep 16, 2022

ArthurSens Sep 16, 2022

mads-hartmann Sep 16, 2022

liam-j-bennett left a comment

iQQBot left a comment

iQQBot commented Sep 16, 2022 •

edited by werft-gitpod-dev-com bot

Loading

Fix running integration tests as part of build #12693

Fix running integration tests as part of build #12693

Conversation

mads-hartmann commented Sep 6, 2022 • edited Loading

Description

Related Issue(s)

How to test

Release Notes

Documentation

Werft options:

mads-hartmann commented Sep 7, 2022 • edited by werft-gitpod-dev-com bot Loading

werft-gitpod-dev-com bot commented Sep 7, 2022

werft-gitpod-dev-com bot commented Sep 14, 2022

werft-gitpod-dev-com bot commented Sep 14, 2022

mads-hartmann commented Sep 14, 2022 • edited by werft-gitpod-dev-com bot Loading

utam0k commented Sep 15, 2022 • edited Loading

geropl left a comment

Choose a reason for hiding this comment

mads-hartmann commented Sep 15, 2022 • edited by werft-gitpod-dev-com bot Loading

mads-hartmann commented Sep 16, 2022

liam-j-bennett Sep 16, 2022 • edited Loading

Choose a reason for hiding this comment

mads-hartmann Sep 16, 2022

Choose a reason for hiding this comment

ArthurSens Sep 16, 2022

Choose a reason for hiding this comment

mads-hartmann Sep 16, 2022

Choose a reason for hiding this comment

liam-j-bennett left a comment

Choose a reason for hiding this comment

iQQBot left a comment

Choose a reason for hiding this comment

iQQBot commented Sep 16, 2022 • edited by werft-gitpod-dev-com bot Loading

mads-hartmann commented Sep 6, 2022 •

edited

Loading

mads-hartmann commented Sep 7, 2022 •

edited by werft-gitpod-dev-com bot

Loading

mads-hartmann commented Sep 14, 2022 •

edited by werft-gitpod-dev-com bot

Loading

utam0k commented Sep 15, 2022 •

edited

Loading

mads-hartmann commented Sep 15, 2022 •

edited by werft-gitpod-dev-com bot

Loading

liam-j-bennett Sep 16, 2022 •

edited

Loading

iQQBot commented Sep 16, 2022 •

edited by werft-gitpod-dev-com bot

Loading