-
Notifications
You must be signed in to change notification settings - Fork 5k
chore: standardise how unit/integration tests is run on CI, adding a build command to compress the system-tests files #26209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
b8d8f85
d8fe22c
ccead60
dd212e2
81d44cd
dddf17c
5fe3f32
5494848
aa87f5f
d1dd718
57b6efb
d5d9f9d
c67f530
229c69d
945983d
26776fa
fcc32be
58e084d
b855019
8b3a8c9
5d6459a
e5265a6
30ccbed
ead2f67
b6dbec3
d4a87cb
218ee9a
70090f6
d7b3d6f
244317c
0e3c931
2b5b183
86ad8ba
b1e8aaa
9d86f71
f914d57
729a432
8678dfc
368417a
0a17e48
d99d0b0
2edf800
f7b150a
2ed629b
7aa8e53
78d203f
6c39510
cf33568
05ab4ab
7bf7938
83996c1
a230581
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -671,7 +671,7 @@ def withBeatsEnv(Map args = [:], Closure body) { | |
| error("Error '${err.toString()}'") | ||
| } finally { | ||
| if (archive) { | ||
| archiveTestOutput(testResults: testResults, artifacts: artifacts, id: args.id, upload: upload) | ||
| archiveTestOutput(directory: directory, testResults: testResults, artifacts: artifacts, id: args.id, upload: upload) | ||
| } | ||
| tearDown() | ||
| } | ||
|
|
@@ -767,6 +767,8 @@ def getCommonModuleInTheChangeSet(String directory) { | |
| * to bypass some issues when working with big repositories. | ||
| */ | ||
| def archiveTestOutput(Map args = [:]) { | ||
| def directory = args.get('directory', '') | ||
|
|
||
| catchError(buildResult: 'SUCCESS', stageResult: 'UNSTABLE') { | ||
| if (isUnix()) { | ||
| fixPermissions("${WORKSPACE}") | ||
|
|
@@ -790,13 +792,20 @@ def archiveTestOutput(Map args = [:]) { | |
| } | ||
| if (args.upload) { | ||
| catchError(buildResult: 'SUCCESS', message: 'Failed to archive the build test results', stageResult: 'SUCCESS') { | ||
| def folder = cmd(label: 'Find system-tests', returnStdout: true, script: 'python .ci/scripts/search_system_tests.py').trim() | ||
| log(level: 'INFO', text: "system-tests='${folder}'. If no empty then let's create a tarball") | ||
| if (folder.trim()) { | ||
| // TODO: nodeOS() should support ARM | ||
| def os_suffix = isArm() ? 'linux' : nodeOS() | ||
| def name = folder.replaceAll('/', '-').replaceAll('\\\\', '-').replaceAll('build', '').replaceAll('^-', '') + '-' + os_suffix | ||
| tarAndUploadArtifacts(file: "${name}.tgz", location: folder) | ||
| withMageEnv(version: "${env.GO_VERSION}"){ | ||
| dir(directory){ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if this is
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We are setting the variable with |
||
| cmd(label: "Archive system tests files", script: 'mage packageSystemTests') | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, let me split the PR in two:
|
||
| } | ||
| } | ||
| def fileName = 'build/system-tests-*.tar.gz' // see dev-tools/mage/target/common/package.go#PackageSystemTests method | ||
| dir("${BASE_DIR}"){ | ||
| cmd(label: "List files to upload", script: "ls -l ${BASE_DIR}/${fileName}") | ||
| googleStorageUploadExt( | ||
| bucket: "gs://${JOB_GCS_BUCKET}/${env.JOB_NAME}-${env.BUILD_ID}", | ||
| credentialsId: "${JOB_GCS_EXT_CREDENTIALS}", | ||
| pattern: "${BASE_DIR}/${fileName}", | ||
| sharedPublicly: true | ||
| ) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,72 @@ | ||||||||||
| // Licensed to Elasticsearch B.V. under one or more contributor | ||||||||||
| // license agreements. See the NOTICE file distributed with | ||||||||||
| // this work for additional information regarding copyright | ||||||||||
| // ownership. Elasticsearch B.V. licenses this file to you under | ||||||||||
| // the Apache License, Version 2.0 (the "License"); you may | ||||||||||
| // not use this file except in compliance with the License. | ||||||||||
| // You may obtain a copy of the License at | ||||||||||
| // | ||||||||||
| // http://www.apache.org/licenses/LICENSE-2.0 | ||||||||||
| // | ||||||||||
| // Unless required by applicable law or agreed to in writing, | ||||||||||
| // software distributed under the License is distributed on an | ||||||||||
| // "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||||||||||
| // KIND, either express or implied. See the License for the | ||||||||||
| // specific language governing permissions and limitations | ||||||||||
| // under the License. | ||||||||||
|
|
||||||||||
| package common | ||||||||||
|
|
||||||||||
| import ( | ||||||||||
| "fmt" | ||||||||||
| "os" | ||||||||||
| "path/filepath" | ||||||||||
| "strings" | ||||||||||
|
|
||||||||||
| devtools "github.com/elastic/beats/v7/dev-tools/mage" | ||||||||||
| ) | ||||||||||
|
|
||||||||||
| // PackageSystemTests packages the python system tests results | ||||||||||
| func PackageSystemTests() error { | ||||||||||
|
Comment on lines
+29
to
+30
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit, if this target packages test results, could it be called
Suggested change
|
||||||||||
| excludeds := []string{".ci", ".git", ".github", "vendor", "dev-tools"} | ||||||||||
|
|
||||||||||
| // include run as it's the directory we want to compress | ||||||||||
| systemTestsDir := filepath.Join("build", "system-tests", "run") | ||||||||||
| files, err := devtools.FindFilesRecursive(func(path string, _ os.FileInfo) bool { | ||||||||||
| base := filepath.Base(path) | ||||||||||
| for _, excluded := range excludeds { | ||||||||||
| if strings.HasPrefix(base, excluded) { | ||||||||||
| return false | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return strings.HasPrefix(path, systemTestsDir) | ||||||||||
| }) | ||||||||||
| if err != nil { | ||||||||||
| return err | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if len(files) == 0 { | ||||||||||
| fmt.Printf(">> there are no system test files under %s", systemTestsDir) | ||||||||||
| return nil | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // create a plain directory layout for all beats | ||||||||||
| beat := devtools.MustExpand("{{ repo.SubDir }}") | ||||||||||
| beat = strings.ReplaceAll(beat, string(os.PathSeparator), "-") | ||||||||||
|
|
||||||||||
| targetFile := devtools.MustExpand("{{ elastic_beats_dir }}/build/system-tests-" + beat + ".tar.gz") | ||||||||||
| parent := filepath.Dir(targetFile) | ||||||||||
| if !fileExists(parent) { | ||||||||||
| fmt.Printf(">> creating parent dir: %s", parent) | ||||||||||
| os.Mkdir(parent, 0750) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| return devtools.Tar(systemTestsDir, targetFile) | ||||||||||
| } | ||||||||||
|
|
||||||||||
| // fileExists returns true if the specified file exists. | ||||||||||
| func fileExists(file string) bool { | ||||||||||
| _, err := os.Stat(file) | ||||||||||
| return !os.IsNotExist(err) | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -35,8 +35,16 @@ stages: | |
| branches: true ## for all the branches | ||
| tags: true ## for all the tags | ||
| stage: extended | ||
| build: | ||
| mage: "mage build test" | ||
| unitTest: | ||
| mage: "mage build unitTest" | ||
| stage: mandatory | ||
| goIntegTest: | ||
| mage: "mage goIntegTest" | ||
| withModule: true | ||
| stage: mandatory | ||
| pythonIntegTest: | ||
| mage: "mage pythonIntegTest" | ||
| withModule: true | ||
|
Comment on lines
+43
to
+47
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, not sure if we can use |
||
| stage: mandatory | ||
| macos: | ||
| mage: "mage build unitTest" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,11 +21,14 @@ stages: | |
| make -C heartbeat update; | ||
| make check-no-changes; | ||
| stage: lint | ||
| build: | ||
| mage: "mage build test" | ||
| unitTest: | ||
| mage: "mage build unitTest" | ||
| stage: mandatory | ||
| goIntegTest: | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think heartbeat doesn't have integration tests. |
||
| mage: "mage goIntegTest" | ||
| stage: mandatory | ||
| macos: | ||
| mage: "mage build test" | ||
| mage: "mage build unitTest" | ||
| platforms: ## override default label in this specific stage. | ||
| - "macosx&&x86_64" | ||
| when: ## Override the top-level when. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can it be done this change in a different PR? Then, we can remove the scripts that configure the tools too?