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 Memcached integration tests #88

Merged
merged 6 commits into from
Nov 5, 2018
Merged

Add Memcached integration tests #88

merged 6 commits into from
Nov 5, 2018

Conversation

labbati
Copy link
Member

@labbati labbati commented Oct 25, 2018

No description provided.

@labbati labbati force-pushed the test/memcached branch 2 times, most recently from 7844191 to 925ff0e Compare October 26, 2018 14:56
Copy link
Contributor

@pawelchcki pawelchcki left a comment

Choose a reason for hiding this comment

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

A few minor comments, and one tiny nitpick.
Otherwise its looking good!

tests/Integration/Integrations/MemcachedTest.php Outdated Show resolved Hide resolved
tests/Integration/Integrations/MemcachedTest.php Outdated Show resolved Hide resolved
});
// A note about xxxMulti from Memcached integration docblock:
//
// setMulti and deleteMulti don't generate out.host and out.port because it
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated PR - but maybe it would worthwhile (if possible) to tag such operation with 'every' used Memcache out.host, out.port pair?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a reminder to investigate this possibility, as it would be an interesting info to add. Thanks for noticing

tests/Integration/Integrations/MemcachedTest.php Outdated Show resolved Hide resolved
@pawelchcki
Copy link
Contributor

Looking at the tests now looks like PHP 5.6 is failing. I wonder if it would be possible to temporarily skip this testsuite only on PHP 5.6 so that its not obscuring other failures.

@labbati
Copy link
Member Author

labbati commented Nov 5, 2018

Back to @pawelchcki for final review:

Copy link
Contributor

@pawelchcki pawelchcki left a comment

Choose a reason for hiding this comment

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

:shipit:

@pawelchcki pawelchcki merged commit c06f3c5 into master Nov 5, 2018
@pawelchcki pawelchcki deleted the test/memcached branch November 5, 2018 16:58
@labbati labbati added this to the 0.3.0 milestone Nov 15, 2018
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