-
Notifications
You must be signed in to change notification settings - Fork 187
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
Refactor integration tests to prepare for proxy-server & agent skew testing #520
Conversation
hc, cc := clientset.HealthyClientsCount(), clientset.ClientsCount() | ||
t.Logf("got %d clients, %d of them are healthy; expected %d", cc, hc, expectedServerCount) | ||
if csc, err := a.GetConnectedServerCount(); err == nil { | ||
t.Logf("got %d server connections; expected %d", csc, expectedServerCount) | ||
} |
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.
Note to reviewers: I dropped ClientsCount
here since it is not exposed by prometheus metrics. It wasn't actually being tested anyway, just used in the error message.
c6ca61e
to
a77097c
Compare
a77097c
to
787d095
Compare
return 0 | ||
} | ||
|
||
func newSingleTimeGetter(m *server.DefaultBackendManager) *singleTimeManager { |
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.
Getting rid of this helps me in a Backend test improvement I started, thanks!
/lgtm Great contribution! /hold Hold in case you want to add that squash directive (some bits like t.Skip, FIXME, etc. show between commits). |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jkh52, tallclair 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 |
/hold cancel I think the |
This PR refactors the running of the test proxy-server & agent to prepare for running them as separate binaries, which will enable version skew testing.
This PR makes it so the server & agent are run with a
RunnerInterface
accessed through a globalFramework
. The runner interfaces returns a respective interface for accessing the testable surface of the server & agent.The
TestConcurrentClientRequest
test needed to be rewritten, since it relied on internal implementation details. The new test is less deterministic, but should still reliably hit the same case being tested (and others).First step towards #519
Next steps:
External{ProxyServer,Agent}Runner
, which takes an external binary to run for the server/agent, and accesses the measurements through prometheus metrics. Expose test flags to select a binary (or the in-tree version).version_skew_test
script that handles fetching target tagged versions, compiling, and running the tests.test-client
binary.ExternalClient
, update version_skew_test to run tests with the external client/assign @jkh52
I recommend reviewing this 1 commit at a time.