-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
[Heartbeat] Zip URL Support #24714
[Heartbeat] Zip URL Support #24714
Conversation
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
Trends 🧪💚 Flaky test reportTests succeeded. Expand to view the summary
Test stats 🧪
|
0a0a19c
to
d1c0917
Compare
Pinging @elastic/uptime (Team:Uptime) |
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.
Looks good to me 👍
err = unzipFile(targetDir, folder, f) | ||
if err != nil { | ||
// TODO: err handlers | ||
os.RemoveAll(targetDir) |
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.
This feels unnecessary right? we already remove the directory when unzip returns error.
) | ||
|
||
func TestZipUrlFetchNoAuth(t *testing.T) { | ||
address, teardown := setupTests() |
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.
I thought doing this for every test was a overkill which is why i moved up everything in to functions and called them from a global test function. I guess it's not? Just a question though.
Adds support for remote zips to browser monitors, letting users just automatically pull from a remote repo. Fixes elastic/uptime#271
Adds support for remote zips to browser monitors, letting users just automatically pull from a remote repo. Fixes elastic/uptime#271
Adds support for remote zips to browser monitors, letting users just automatically pull from a remote repo. Fixes elastic/uptime#271
@andrewvc One of the new tests in the PR is panicking in master.
|
* [Heartbeat] Zip URL Support (#24714) Adds support for remote zips to browser monitors, letting users just automatically pull from a remote repo. Fixes elastic/uptime#271 * Fix null check * Add missing test
@andrewkroh fixed it in #25288 , apologies for the flakiness! |
Adds support for remote zips to browser monitors, letting users just automatically pull from a remote repo.
Fixes elastic/uptime#271
Why is it important?
Checklist
- [ ] I have made corresponding changes to the documentationCHANGELOG.next.asciidoc
orCHANGELOG-developer.next.asciidoc
.Author's Checklist
How to test this PR locally
Related issues
Use cases
Screenshots
Logs