This repository has been archived by the owner on Sep 17, 2024. It is now read-only.
[CI] tear down the workspace #885
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
v1v
commented
Mar 15, 2021
mdelapenya
reviewed
Mar 15, 2021
| /** | ||
| * Tear down the setup for the static workers. | ||
| */ | ||
| def tearDown(Map args = [:]){ |
There was a problem hiding this comment.
Wdyt about having a specific step in the library for cleaning up Go dependencies?
There was a problem hiding this comment.
IMO the build system should be the one ensuring the workspace is in good shape. Then the CI should only caring to delete the workspace if required.
Though, it seems go has got some opinionated approach and the build system does not provide those tools, based on that, we could create a specific step in the library to help with.
There are some questions regarding:
- the new step will always run within the
Goenvironmental context? - if running as a post declarative step, will it work?
Do you think we just need to raise an issue regarding this improvement?
There was a problem hiding this comment.
I was/am concerned about having the very same method in different pipelines (Beats and here) and probably in many of the other Go projects that we maintain.
I agree that the build system should keep the workspace and dependencies in a good shape, but as you know there is nothing as maven/gradle to do so.
Regarding your questions:
- I think the Go environment should be the same, just in case the
cleanmethod could change between different Go versions - I've been reading about
go clean. Do you think it could be a best practice to invoke it before tests run? The same we domvn clean testin the Maven world. If so then we should not think about the post step issue, as the environment would always be right. Even better, we could enforce the clean in the set Go env step 🤔
💔 Tests Failed
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪Test errors
Expand to view the tests failures
|
mdelapenya
approved these changes
Mar 15, 2021
cachedout
approved these changes
Mar 15, 2021
|
BTW, the step in the pipeline shows this message:
Is it related? |
|
It's now much better with 4630b71
|
mdelapenya
approved these changes
Mar 15, 2021
v1v
commented
Mar 15, 2021
| error(e.toString()) | ||
| } finally { | ||
| junit(allowEmptyResults: true, keepLongStdio: true, testResults: "${BASE_DIR}/outputs/TEST-*.xml") | ||
| } finally { |
There was a problem hiding this comment.
We only need the finally, then error is redundant
v1v
added a commit
to v1v/e2e-testing
that referenced
this pull request
Mar 15, 2021
v1v
added a commit
to v1v/e2e-testing
that referenced
this pull request
Mar 15, 2021
v1v
added a commit
to v1v/e2e-testing
that referenced
this pull request
Mar 15, 2021
v1v
added a commit
to v1v/e2e-testing
that referenced
this pull request
Mar 15, 2021
v1v
added a commit
to v1v/e2e-testing
that referenced
this pull request
Mar 15, 2021
…hings * upstream/master: [CI] tear down the workspace (elastic#885) docs: add Make as build system (elastic#886) fix: proper usage of step (elastic#883) feat: run most frequent flavours in the PR stage (elastic#873) break: move from "pull-requests" to "commits" GCP bucket (elastic#866) fix: use proper flags (elastic#868) feat: instrument Helm charts test suite (elastic#858)
v1v
added a commit
that referenced
this pull request
Mar 16, 2021
v1v
added a commit
that referenced
this pull request
Mar 16, 2021
v1v
added a commit
that referenced
this pull request
Mar 16, 2021
8 tasks
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Delete workspace
Why is it important?
If we use static/permanent workers then we might suffer disk space issues
Checklist
make noticein the proper directory)Author's Checklist
How to test this PR locally
Related issues
Closes #884
#707 can benefit from this :)