这是indexloc提供的服务，不要输入任何密码
Skip to content

remove mako tests and dependency #12266

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 12, 2021
Merged

remove mako tests and dependency #12266

merged 1 commit into from
Nov 12, 2021

Conversation

nader-ziada
Copy link
Member

Proposed Changes

  • Remove old performance tests using discontinued the Mako tool
  • Remove Mako dependency

Release Note

Remove performance tests using Mako

@google-cla google-cla bot added the cla: yes Indicates the PR's author has signed the CLA. label Nov 11, 2021
@knative-prow-robot knative-prow-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. area/test-and-release It flags unit/e2e/conformance/perf test issues for product features labels Nov 11, 2021
@nader-ziada
Copy link
Member Author

/area autoscale

@codecov
Copy link

codecov bot commented Nov 11, 2021 •
edited
Loading

Codecov Report

Merging #12266 (eefc3cc) into main (77ba3bd) will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #12266      +/-   ##
==========================================
- Coverage   87.34%   87.25%   -0.10%     
==========================================
  Files         195      195              
  Lines        9583     9600      +17     
==========================================
+ Hits         8370     8376       +6     
- Misses        934      941       +7     
- Partials      279      283       +4
Impacted Files Coverage Δ
pkg/autoscaler/statforwarder/forwarder.go 90.74% <0.00%> (-5.56%) ⬇️
pkg/autoscaler/statforwarder/processor.go 88.88% <0.00%> (-5.56%) ⬇️
pkg/apis/config/defaults.go 87.83% <0.00%> (-4.10%) ⬇️
pkg/apis/serving/k8s_validation.go 93.49% <0.00%> (-0.02%) ⬇️
cmd/queue/main.go 0.49% <0.00%> (-0.01%) ⬇️
pkg/apis/serving/v1/revision_defaults.go 97.22% <0.00%> (+0.16%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77ba3bd...eefc3cc. Read the comment docs.

@nader-ziada nader-ziada mentioned this pull request Nov 11, 2021
Closed
markusthoemmes
markusthoemmes approved these changes Nov 11, 2021
View reviewed changes
Copy link
Contributor

@markusthoemmes markusthoemmes left a comment

Choose a reason for hiding this comment

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

/lgtm
/approve

/hold
/assign @julz @dprotaso

Just to get more +1s

@knative-prow-robot knative-prow-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 11, 2021
@knative-prow-robot knative-prow-robot added the lgtm Indicates that a PR is ready to be merged. label Nov 11, 2021
@knative-prow-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: markusthoemmes, nader-ziada

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@knative-prow-robot knative-prow-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 11, 2021
@mgencur
Copy link
Contributor

mgencur commented Nov 12, 2021

The whole "test/performance" folder is now removed with this PR. I'm just curious if there's a different approach/tool that is going to be used for performance testing. Thanks

@markusthoemmes
Copy link
Contributor

@mgencur Yes, kperf is being extended by Nader as part of the autoscaling group to get performance tests that can be run anywhere and by anyone.

@nader-ziada
Copy link
Member Author

I also should have mentioned in the PR description, the Mako tool is not being developed anymore and these test haven't been run for a few months now.

@dprotaso
Copy link
Member

🎉
/hold cancel

If you haven't already we should make sure we stop prow from running these performance tests

@knative-prow-robot knative-prow-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Nov 12, 2021
@nader-ziada
Copy link
Member Author

If you haven't already we should make sure we stop prow from running these performance tests

Chi already removed them from test-infra a while back knative/test-infra#2937

@julz
Copy link
Member

julz commented Nov 12, 2021

If a performance test falls in the forest and no-one is around to hear it..

+1 from me too

@knative-prow-robot knative-prow-robot merged commit 24766a5 into knative:main Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@markusthoemmes markusthoemmes markusthoemmes approved these changes

Assignees

@dprotaso dprotaso

@julz julz

@markusthoemmes markusthoemmes

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. area/autoscale area/test-and-release It flags unit/e2e/conformance/perf test issues for product features cla: yes Indicates the PR's author has signed the CLA. lgtm Indicates that a PR is ready to be merged. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@nader-ziada @knative-prow-robot @mgencur @markusthoemmes @dprotaso @julz