Run tekton tasks tests in parallel #432

henninggross · 2022-02-03T13:37:23Z

This PR introduces running the Tekton tasks tests in parallel.
Due to the fact that the test outputs of the different test cases are interleaved we came up with a solution to log the outputs of each test case to their respective log file inside test/testdata/test-results.
This directory will be uploaded as an artifact after the test steps for the developer to be able to check the logs conveniently.

Ref #401
Closes #14

Tasks:

Updated design documents in docs/design directory or not applicable
Updated user-facing documentation in docs directory or not applicable
Ran tests (e.g. make test) or not applicable
Updated changelog or not applicable

Container logs printed using fmt.Println which is test agnostic, leading to outputs occassionally being associated with the wrong testcase whenever tests run in parallel

henninggross · 2022-02-03T15:13:29Z

Now that the tests have successfully run through we calculated that the test duration was cut down to 19m54s from 23m38s (only comparing one test run with another, though).

henrjk

Looks awesome to me. I am curious what you say to my previous comments.

Could this change be in the way to introduce caching (see proposal PR #412 ) as there might be concurrent access to the cache which could potentially cause flaky tests.

henrjk · 2022-02-04T08:37:19Z

internal/testoutput/disentangle.go

+				return fmt.Errorf("failed to create directory: %w", err)
+			}
+
+			f, err := os.OpenFile(filePath, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0644)


I am curious did you need this file flags and permissions? Have you different flags/permissions first - perhaps os.Create() and it did not work?

Good catch, we'll check os.Create() instead, looks like this is a convenience wrapper around os.OpenFile()

henrjk · 2022-02-04T08:41:36Z

internal/testoutput/disentangle.go

+			fileHandles[event.Test] = f
+		case "pause", "cont", "skip": // do nothing
+		case "pass", "fail":
+			f, ok := fileHandles[event.Test]


Should there be some error reporting if it is not ok?

henrjk · 2022-02-04T08:43:12Z

internal/testoutput/disentangle.go

+		case "pass", "fail":
+			f, ok := fileHandles[event.Test]
+			if ok {
+				err := f.Close()


I assume test pass and fail is typically captured in prior output. Perhaps its worthwhile to nonetheless write this explicitly in the log.

@henrjk you're spot-on, go test actually does write a pass/fail marker with an "bench"/"output" event.Action.

The object for events of type pass and fail do not have any output. The output is captured in the prior object with event type output like this:

--- PASS: TestTaskODSBuildGradle/ods-build-gradle/task_should_build_gradle_app (70.27s)

So the only additional log that we could provide reacting on pass and fail would be a duplicate of this one and that's why @kuebler and I decided to do nothing but closing the file on those.

henrjk · 2022-02-04T08:44:10Z

internal/testoutput/disentangle.go

+				return fmt.Errorf("failed to open output file: %w", err)
+			}
+			fileHandles[event.Test] = f
+		case "pause", "cont", "skip": // do nothing


Is skip of interest in the log?

Similarly to #432 (comment) the output for a skipped test would be captured in the prior output so it would only lead to us printing information twice if we react on skip, pause or cont.

michaelsauter · 2022-02-04T13:11:47Z

Really, really great work!

I have not looked into the details, but I have two more general questions:

Would you think it makes sense to provide a parallel and a non-parallel execution of the tests?
The test time savings are not bad, but I hoped for more tbh. I realise this is likely because the test files still run sequentially, only the test cases within one test func run in parallel. Have you thought about structuring this differently so that you can basically pick and choose how many tests of the overall set of tests you can run in parallel? Not sure how many in parallel GitHub Actions would support but maybe we are not pushing the limits here yet ;)

kuebler · 2022-02-04T13:38:45Z

Would you think it makes sense to provide a parallel and a non-parallel execution of the tests?

Good idea, probably the tests read an ENV variable and based on that decide whether to execute the tests in parallel or not.

The test time savings are not bad, but I hoped for more tbh. I realise this is likely because the test files still run sequentially, only the test cases within one test func run in parallel. Have you thought about structuring this differently so that you can basically pick and choose how many tests of the overall set of tests you can run in parallel? Not sure how many in parallel GitHub Actions would support but maybe we are not pushing the limits here yet ;)

Good catch, let's try and put t.Parallel() in the test functions as well and see if that improves the execution time. In addition, we could also take a look whether we can squeeze out a bit of time out of the ODS image builds. E.g. the helm image installs sops and age using go install, however, both also publish binary releases on Github which we could just download and copy like we do for helm itself.

michaelsauter · 2022-02-04T13:43:30Z

E.g. the helm image installs sops and age using go install, however, both also publish binary releases on Github which we could just download and copy like we do for helm itself.

Careful, this is on purpose: #364 (though we can also change the approach if we find a good alternative). Different topic than parallel testing anyway ;)

kuebler · 2022-02-04T14:06:13Z

Careful, this is on purpose: #364 (though we can also change the approach if we find a good alternative). Different topic than parallel testing anyway ;)

age, however, in contrast to sops does release arm64 builds as well; we could at least download the binaries for age? But you're right: This is more about #401 than #14 😄

- previously, only table tests were run in parallel - in addition, check whether running the parent tasks themselves in parallel further improves overall test execution time

…com/opendevstack/ods-pipeline into feature/improve-test-execution-time

- looks like it doesn't have possitive effect on test execution time - makes tests flaky as they're now running into all kinds of timeouts due to concurrent execution

michaelsauter · 2023-09-29T07:45:54Z

Closing as a consequence of #722. The tests are way faster now because there is less to test in this repo, hence the need for parallel tests is less.

henninggross and others added 8 commits January 31, 2022 15:12

initial version of parallel tests and individual log files

d179c4f

WIP parallel test cases

027c327

Log container outputs using t.Logf/t.Log

a3c7665

Container logs printed using fmt.Println which is test agnostic, leading to outputs occassionally being associated with the wrong testcase whenever tests run in parallel

always overwrite log files with new test output

175a938

remove debug print

367ae66

introduce parallel tests in github actions

f9eccc4

Merge branch 'master' into feature/improve-test-execution-time

a8c7ce4

add parallel flag to test-tasks make target

68f0d0e

henninggross added the skip changelog label Feb 3, 2022

henninggross requested review from michaelsauter, gerardcl and henrjk February 3, 2022 13:37

henninggross added 4 commits February 3, 2022 14:41

fix linting

1ad3d87

🥴 fix linting

54a456c

✏️ fix typo

157f71b

❌ remove parallel flag from go test command

d861f65

remove unused TestCase parameter

33f095e

henrjk reviewed Feb 4, 2022

View reviewed changes

kuebler added 6 commits February 7, 2022 08:01

Try and run top level task tests in parallel

bdbd8ba

- previously, only table tests were run in parallel - in addition, check whether running the parent tasks themselves in parallel further improves overall test execution time

Merge branch 'feature/improve-test-execution-time' of https://github.…

66f0828

…com/opendevstack/ods-pipeline into feature/improve-test-execution-time

Remove accidentally commited file

31b2d8c

Try and set -parallel explicitly in GH actions

e3cbb1e

Always upload test logs, even if steps failed

4d6c107

Roll back parallel parent test execution

83c19cd

- looks like it doesn't have possitive effect on test execution time - makes tests flaky as they're now running into all kinds of timeouts due to concurrent execution

michaelsauter closed this Sep 29, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Run tekton tasks tests in parallel #432

Run tekton tasks tests in parallel #432

Uh oh!

henninggross commented Feb 3, 2022

Uh oh!

henninggross commented Feb 3, 2022

Uh oh!

henrjk left a comment

Uh oh!

henrjk Feb 4, 2022

Uh oh!

kuebler Feb 7, 2022

Uh oh!

henrjk Feb 4, 2022

Uh oh!

henrjk Feb 4, 2022

Uh oh!

kuebler Feb 7, 2022 •

edited

Loading

Uh oh!

henninggross Feb 7, 2022 •

edited

Loading

Uh oh!

henrjk Feb 4, 2022

Uh oh!

henninggross Feb 7, 2022

Uh oh!

michaelsauter commented Feb 4, 2022 •

edited

Loading

Uh oh!

kuebler commented Feb 4, 2022

Uh oh!

michaelsauter commented Feb 4, 2022

Uh oh!

kuebler commented Feb 4, 2022 •

edited

Loading

Uh oh!

michaelsauter commented Sep 29, 2023

Uh oh!

Uh oh!

Run tekton tasks tests in parallel #432

Run tekton tasks tests in parallel #432

Uh oh!

Conversation

henninggross commented Feb 3, 2022

Uh oh!

henninggross commented Feb 3, 2022

Uh oh!

henrjk left a comment

Choose a reason for hiding this comment

Uh oh!

henrjk Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

kuebler Feb 7, 2022

Choose a reason for hiding this comment

Uh oh!

henrjk Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

henrjk Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

kuebler Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

henninggross Feb 7, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

henrjk Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

henninggross Feb 7, 2022

Choose a reason for hiding this comment

Uh oh!

michaelsauter commented Feb 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kuebler commented Feb 4, 2022

Uh oh!

michaelsauter commented Feb 4, 2022

Uh oh!

kuebler commented Feb 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

michaelsauter commented Sep 29, 2023

Uh oh!

Uh oh!

kuebler Feb 7, 2022 •

edited

Loading

henninggross Feb 7, 2022 •

edited

Loading

michaelsauter commented Feb 4, 2022 •

edited

Loading

kuebler commented Feb 4, 2022 •

edited

Loading