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

Add unit tests #70

Merged
merged 2 commits into from
Sep 10, 2018
Merged

Add unit tests #70

merged 2 commits into from
Sep 10, 2018

Conversation

timrourke
Copy link
Contributor

@timrourke timrourke commented Sep 1, 2018

This PR installs and configures PHPUnit. In addition, it adds unit tests.

Thanks, looking forward to your feedback!

EDIT: I am now running PHPUnit tests in Travis. However, in order to support versions of PHP all the way back to PHP 5.3, I installed a rather old version of PHPUnit. This isn't ideal; some code in the older version of PHPUnit calls deprecated functions. See https://travis-ci.org/DataDog/php-datadogstatsd/jobs/423273440 for an example of the deprecation warnings.

If those deprecation warnings seem problematic, PHPUnit could be upgraded to a more recent version (say, to 5.7.x, which supports PHP5.6+), and then Travis failures could be suppressed on a per-version basis. For example: https://github.com/sebastianbergmann/phpunit/blob/master/.travis.yml#L16

@timrourke timrourke force-pushed the feature/add-unit-tests branch from 51ab835 to e478455 Compare September 1, 2018 03:08
@timrourke timrourke force-pushed the feature/add-unit-tests branch 6 times, most recently from 67c5e67 to 8c3da37 Compare September 5, 2018 14:09
@timrourke
Copy link
Contributor Author

🎉 100% loc are now covered as can be seen in this build's output.

I know this PR is quite large, so do feel free to request that I break it into smaller pieces for easier review if that would be useful to you.

Thanks!

Add tests for DogStatsd::flush

Add tests for DogStatsd::report and Dogstatsd::service_check

Extract method

Improve test coverage for DogStatsd::service_check

Add test for DogStatsd::send

Run PHPUnit tests in Travis

Use simpler composer install CLI options

Downgrade PHPUnit to support PHP 5.3 :(

Add test for DogStatsd::timing

Add test for DogStatsd::microtiming

Add tests for DogStatsd::gauge and DogStatsd::histogram

Correct histogram test

Add test for DogStatsd::distribution

Add test for DogStatsd::set

Add composer script to run tests

Add coverage for DogStatsd::serialize_tags

Add test for DogStatsd::updateStats

Add tests for DogStatsd::increment and DogStatsd::decrement

Improve test coverage for DogStatsd::updateStats

Add test for DogStatsd::eventUdp

Add coverage for Dogstatsd::eventUdp

Add coverage for DogStatsd::testSend with sample rates < 1

Add tests for BatchedDogStatsd

Add spy for curl functions

Give socket spy global specific name

Move tests into namespace

Improve namespace organization

Reorganize tests for clarity

Add tests for curl DogStatsd::event happy path

Add stub for error_log

Add tests for curl error states
@timrourke
Copy link
Contributor Author

@hush-hush could you take a look at this PR? i see that this library continues to evolve and i believe it deserves test coverage to aid future development efforts and safeguard users against possible bugs.

Copy link
Contributor

@masci masci left a comment

Choose a reason for hiding this comment

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

This is great, I'm not a php expert but 100% coverage instead of 0% needs no discussion!

@masci masci merged commit 3da3124 into DataDog:master Sep 10, 2018
@AC-TimRourke
Copy link

AC-TimRourke commented Sep 10, 2018

🎉 thanks @masci !!

Edit: Whoops, logged in with the wrong GitHub account! lol

@timrourke timrourke deleted the feature/add-unit-tests branch September 10, 2018 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants