Skip to content
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

Performance tool can accept test MRs via URLs #130

Merged
merged 2 commits into from
Aug 29, 2023
Merged

Conversation

Piotr1215
Copy link

@Piotr1215 Piotr1215 commented Aug 21, 2023

Description of your changes

Managed resources used for performance testing are read from disk and passed as arguments to the CLI. This commit introduces changes to the CLI so that test MRs can now be passed via URLs.

This change is needed to add more flexible test scenarios for the performance CI pipeline.

Fixes https://github.com/upbound/team-extensions/issues/177

I have:

  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Locally running the tool with URL MRs

@Piotr1215 Piotr1215 force-pushed the perf-test-url-input branch 3 times, most recently from 821171e to 2cd9c8e Compare August 21, 2023 15:36
Managed resources used for performance testing are read from disk and
passed as arguments to the CLI. This commit introduces changes to the
CLI so that test MRs can now be passed via URLs.

This change is needed to add more flexible test scenarios for the
performance CI pipeline.
@Piotr1215 Piotr1215 force-pushed the perf-test-url-input branch from 2cd9c8e to bf5fd9b Compare August 21, 2023 15:44
Copy link
Member

@sergenyalcin sergenyalcin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Piotr1215 LGTM! I left a nit comment.

return nil, errors.Wrap(err, "cannot fetch URL")
}
defer func() {
_ = resp.Body.Close()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: What about handling this error?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the suggestion @sergenyalcin , i've handled the error.

@Piotr1215 Piotr1215 force-pushed the perf-test-url-input branch from 2ac9129 to cf073f6 Compare August 29, 2023 13:05
@Piotr1215 Piotr1215 merged commit 0807fae into main Aug 29, 2023
5 checks passed
@Piotr1215 Piotr1215 deleted the perf-test-url-input branch August 29, 2023 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants