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

Use a separate mock server for benchmarks #450

Merged
merged 1 commit into from May 3, 2017
Merged

Conversation

jchambers
Copy link
Owner

This has two good outcomes:

  1. Because we've removed a lot of logic, it's less likely that we'll be inadvertently benchmarking the server instead of the client.
  2. We don't have to worry about performance as much for the mock server we use for testing; this frees us to write code that is clearer and simpler (and ideally more likely to be correct), but maybe a little slower.

This has two good outcomes:

1. Because we've removed a lot of logic, it's less likely that we'll be inadvertently benchmarking the server instead of the client.
2. We don't have to worry about performance as much for the mock server we use for testing; this frees us to write code that is clearer and simpler (and ideally more likely to be correct), but maybe a little slower.
@jchambers jchambers added this to the v0.10 milestone Apr 16, 2017
@jchambers
Copy link
Owner Author

Would anybody like this benchmark server to live in the main pushy module (as opposed to the benchmark module)? I'm inclined to leave it out to keep the number of public classes small, but could certainly be persuaded otherwise if "send notifications to a thing that always says yes" is a common use case in integration testing.

@n8leon
Copy link

n8leon commented Apr 25, 2017

👍 :-)
Yup, will certainly be helpful for us to have a sink in order to load test our own code.
Thx!

@jchambers
Copy link
Owner Author

To really make this public in a non-crazy way, I think we need to abstract out some of the existing mock server stuff and refactor some things. For now, I'm going to merge this as-is to clear the way for #451, but with the understanding that we'll revisit the public benchmark server thing once that's under control.

I've filed #459 so we don't lose track of the idea.

@jchambers
Copy link
Owner Author

I'm not sure what's going wrong with test status reporting; the build certainly passed, so I'm going to merge this and figure out what's up with Travis as a separate effort.

@jchambers jchambers merged commit 2e2ecc0 into master May 3, 2017
@jchambers jchambers deleted the benchmark_server branch May 3, 2017 00:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants