-
Notifications
You must be signed in to change notification settings - Fork 16
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
Selectively skip and fix tests after PR#212 #235
Conversation
PR #212 made some changes on `main` that require the presence of `MANIFEST_URL`. However, the pipeline is still configured to run the same script when a PR (from an upstream branch) is raised and it currently fails. This commit skips tests when triggered from another pipeline (unified release), and allows running tests when triggered by PR creation by picking up a suitable `MANIFEST_URL`.
This is ready for review. Because of the multiple ways that this pipeline may run -- via unified release trigger, via
Example run: https://buildkite.com/elastic/elastic-stack-installers/builds/3720 using this PR |
@strawgate the tests seem to be failing recently with any thoughts on this? |
That's just the debug output for a negative test case. The failing test case is:
Looking into this |
…nt standalone messages
Pushed a commit, i think it was just a race condition between the agent starting up and us checking its log file |
Build failed with:
re-running build now |
This is an unrelated failure in the DRA publish step: we allow running the publish step in dry run mode from within PRs, but it seems to me that we shouldn't: the unified release expected certain hard coded directories (e.g. |
@strawgate FYI we are still having failures related to tests after your latest commits: https://buildkite.com/elastic/elastic-stack-installers/builds/3747 |
Yep will look today, my last commit was to add some additional debug info |
It looks like the last test run passed, will rerun a couple of times to make sure it's not flaky |
@strawgate your latest commit seems to have fixed the tests: https://buildkite.com/elastic/elastic-stack-installers/builds/3750#018d7eb8-cb49-4989-ad1b-f5314ec4f0d6
Sounds like a great idea! |
We got another fresh failure in https://buildkite.com/elastic/elastic-stack-installers/builds/3751#018d7ee0-3fea-4c2a-abfd-0d6f6f6969e3 |
… fix race condition
fingers crossed on this one -- if this last change doesn't fix it i'll disable the test case that triggers the race condition and fix it in a separate PR |
… standalone checks
Tests passed with latest change, running another retest to check for flaky tests |
@strawgate I think we are good here. I just triggered one more build to check for flakiness: https://buildkite.com/elastic/elastic-stack-installers/builds/3794 could you provide a final review so that we can merge? Also @amitkanfer could please take a look again? |
buildkite test this |
done! |
PR #212 made some changes on
main
that require the presence ofMANIFEST_URL
. However, the pipeline is still configured to run the same script when a PR (from an upstream branch) is raised and it currently fails.This commit skips tests when triggered from another pipeline (unified release), and allows running tests when triggered by PR creation by picking up a suitable
MANIFEST_URL
.