search

vthiery / cpp-statsd-client

A header-only StatsD client implemented in C++
MIT License
51 stars 19 forks source link

End to End Testing #19

Closed kevinkreiser closed 3 years ago

kevinkreiser commented 3 years ago

fixes #18

This PR expands the test application that we run in CI so that it spawns a thread which listens on the statsd UDP port and stores the messages it got in memory. Then we validate that the messages it received were the ones we expected it to.

I made quite a couple changes here to make this more testable:

  1. Change the code to use c++11 style random number generation and add seeding of the RNG to the client interface (this allows repeatable results in the test)
  2. Add a mock statsd server implementation to the test directory which we throw in a background thread to collect the messages it receives for validation

There were other changes I made as nice-to-haves:

  1. Rather than using a char[] buffer I use a string directly, this will allow us to avoid copying the buffer into a string when it goes on to the UDPSender, in the future we can make this more efficient by making it a member and keeping it a fixed size
  2. I put a bunch of TODOs as questions in the code or suggestions, please let me know which ones i should implement or remove

Another side question I have is, should we add some semver to the API? I've been changing the main methods here and there and whenever you make your next release I suppose the version should reflect that. I think having the version number in code might be helpful to others as well.

vthiery commented 3 years ago

Another side question I have is, should we add some semver to the API? I've been changing the main methods here and there and whenever you make your next release I suppose the version should reflect that. I think having the version number in code might be helpful to others as well.

If we break the API, I'll bump the release version to the next major, so 2.0.0.

kevinkreiser commented 3 years ago

@vthiery thanks for all the review feedback i'll get them all changed and break out the other stuff into another PR. should have that doen shortly!

kevinkreiser commented 3 years ago

i believe ive removed all of the additional changes here and have down to just the changes discussed in the first round of review:

  1. random number gen change to allow for testing
  2. cap frequency to valid range
  3. vastly expanded testing
  4. help methods for determining an unusable socket (invalid file descriptor)
kevinkreiser commented 3 years ago

@vthiery i got master merged in with the linting PR, i think this is ready for review again

kevinkreiser commented 3 years ago

@vthiery OK I feel pretty sure now that I've covered all of the things discussed in the review comments. Sorry for taking such a long time on that one!