Skip to content
This repository has been archived by the owner on May 2, 2023. It is now read-only.

Fix performance test and integration test #185

Merged
merged 1 commit into from
Oct 14, 2022
Merged

Fix performance test and integration test #185

merged 1 commit into from
Oct 14, 2022

Conversation

jwcesign
Copy link
Member

@jwcesign jwcesign commented Oct 13, 2022 •
edited
Loading

Signed-off-by: jwcesign [email protected]

Changes

  • 🐛 Fix bug

/kind bug

Fixes #none

Release Note

none

Docs

none

@knative-prow knative-prow bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. kind/bug Categorizes issue or PR as related to a bug. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 13, 2022
@jwcesign
Copy link
Member Author

/test performance-tests

@codecov
Copy link

codecov bot commented Oct 13, 2022 •
edited
Loading

Codecov Report

Merging #185 (b7eeb1f) into main (39b45a2) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #185   +/-   ##
=======================================
  Coverage   67.35%   67.35%           
=======================================
  Files           5        5           
  Lines         193      193           
=======================================
  Hits          130      130           
  Misses         44       44           
  Partials       19       19

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jwcesign
Copy link
Member Author

/test performance-tests

@jwcesign
Copy link
Member Author

/test performance-tests

@jwcesign
Copy link
Member Author

/test performance-tests

@knative-prow knative-prow bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 13, 2022
@jwcesign
Copy link
Member Author

/test performance-tests

@knative-prow knative-prow bot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Oct 13, 2022
@jwcesign
Copy link
Member Author

/test performance-tests

@jwcesign
Copy link
Member Author

jwcesign commented Oct 13, 2022 •
edited
Loading

Should we really need to copy these files from serving repo? how about: git clone serving repo in script, cd to right dir, run it directly to launch the cluster?

Because it needs efforts to care and sync from serving repo when there is updates.
cc @psschwei

@psschwei
Copy link
Contributor

Common scripts should go in https://github.com/knative/hack , which has automation to handle syncing across repos.

Do we really need copies of all the serving scripts to run these performance tests?

@knative-prow knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Oct 14, 2022
@jwcesign
Copy link
Member Author

/test performance-tests

@jwcesign
Copy link
Member Author

/test performance-tests

@jwcesign
Copy link
Member Author

/test performance-tests

@jwcesign jwcesign force-pushed the performance-test branch 2 times, most recently from 2dacf25 to f430719 Compare October 14, 2022 06:21
@knative-prow knative-prow bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 14, 2022
@jwcesign
Copy link
Member Author

/test performance-tests

@jwcesign
Copy link
Member Author

/test integration-tests

@jwcesign jwcesign force-pushed the performance-test branch 3 times, most recently from ae26a48 to a9b9a7c Compare October 14, 2022 07:28
jwcesign
jwcesign commented Oct 14, 2022
View reviewed changes
.github/workflows/kubeadm-e2e-crio.yaml
@@ -12,7 +12,7 @@ defaults:

env:
GOTESTSUM_VERSION: 1.7.0
CRIO_VERSION: 1.24.1
CRIO_VERSION: 1.25.1
Copy link
Member Author

Choose a reason for hiding this comment

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

sometimes the case will fail, they fixed here: cri-o/cri-o#6122

@jwcesign
Copy link
Member Author

/test performance-tests

@jwcesign
Copy link
Member Author

Now the test result is ok: https://gcsweb.knative.dev/gcs/knative-prow/pr-logs/pull/knative-sandbox_container-freezer/185/performance-tests_container-freezer_main/1580825153942589440/artifacts/hey/
cc @psschwei

@jwcesign jwcesign changed the title [WIP] Fix performance test Fix performance test Oct 14, 2022
@knative-prow knative-prow bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Oct 14, 2022
@jwcesign jwcesign changed the title Fix performance test Fix performance test and integration test Oct 14, 2022
@jwcesign
Copy link
Member Author

Now the output is csv, I am asking whether it has plan to support html output: rakyll/hey#287

@psschwei
Copy link
Contributor

/test performance-tests

psschwei
psschwei reviewed Oct 14, 2022
View reviewed changes
pkg/freeze/crio/pause_test.go Outdated
criSocketPath := test.GetRandomSocketPath()
criServer := test.NewCriRuntimeServer()
go test.RunCriServer(criServer, criSocketPath)
t.Logf("Create cri sock path:%s", criSocketPath)
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure we need to log this or the crio one for a test unless the test fails, so maybe it would be better to add this to the error (if it's necessary)?

Copy link
Member Author

Choose a reason for hiding this comment

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

The ut will fail sometimes, add log for better find it out the reason, actually not need it, I delete it.

@knative-prow knative-prow bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 14, 2022
@jwcesign
Fix performance test
b7eeb1f 
Signed-off-by: jwcesign <[email protected]>
psschwei
psschwei reviewed Oct 14, 2022
View reviewed changes
Copy link
Contributor

@psschwei psschwei 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

@knative-prow knative-prow bot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2022
@knative-prow
Copy link

knative-prow bot commented Oct 14, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: jwcesign, psschwei

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 knative-prow bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Oct 14, 2022
@knative-prow knative-prow bot merged commit 15fac37 into knative-extensions:main Oct 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@psschwei psschwei psschwei left review comments

Assignees

@psschwei psschwei

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. kind/bug Categorizes issue or PR as related to a bug. lgtm Indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jwcesign @psschwei