Skip to content

ensure tests are ok for 32bits build #319

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

Merged
merged 1 commit into from
Feb 9, 2017

Conversation

remicollet
Copy link
Collaborator

On both i386 and x86_64

=====================================================================
TEST RESULT SUMMARY
---------------------------------------------------------------------
Exts skipped    :    0
Exts tested     :   18
---------------------------------------------------------------------

Number of tests :   81                78
Tests skipped   :    3 (  3.7%) --------
Tests warned    :    0 (  0.0%) (  0.0%)
Tests failed    :    0 (  0.0%) (  0.0%)
Expected fail   :    1 (  1.2%) (  1.3%)
Tests passed    :   77 ( 95.1%) ( 98.7%)
---------------------------------------------------------------------
Time taken      :   39 seconds
=====================================================================

@laruence laruence merged commit 4104f40 into php-memcached-dev:master Feb 9, 2017
@laruence
Copy link
Contributor

laruence commented Feb 9, 2017

thanks :)

@remicollet remicollet deleted the issue-tests branch February 9, 2017 06:38
@remicollet
Copy link
Collaborator Author

remicollet commented Feb 9, 2017

Additional information:

I just run a test build in Fedora rawhide: https://koji.fedoraproject.org/koji/taskinfo?taskID=17684839

Test suite passes on all primary arches: i686, x86_64, ppc64, ppc64le, aarch64 and armv7hl :)

@sodabrew
Copy link
Contributor

sodabrew commented Feb 9, 2017

You're no longer consistently testing that 64-bit works on 64-bit systems, which was the point of that test, which tested the code that I added to resolve the problem that 64-bit offsets didn't work, which was the original user issue in #156 and #306.

I prefer to avoid testing 64-bits randomly; I appreciate that "most of the time" rand will end up in the upper bit range on a 64-bit system, but that's not going to consistently catch a bug if one is introduced.

A more appropriate fix, to me, would be skipping the 64-bit tests based on the value of the PHP_INT_MAX constant.

@remicollet
Copy link
Collaborator Author

Or duplicate the test, keeping current one for 32/64 incr, and the new ones for 64bist incr, skipped when PHP_INT_SIZE==4 ?

@sodabrew
Copy link
Contributor

sodabrew commented Feb 9, 2017

I was thinking about duplicating the test, too, otherwise it is hidden whether the 64-bits really got exercised. I thought the line about the mt_rand_max() was picking an offset to test, but that was totally unrelated, so, yeah, definitely let's work out some agreeable way to test within the range of available integers.

Related: the CAS tokens are always 64-bits, and there is a fallback to strings if the number is larger than the local PHP can handle. Does that many any sense to do here? Or would it just blow up people's code if they suddenly started getting strings back where they expected integers.

@remicollet
Copy link
Collaborator Author

I was thinking about duplicating the test, too,

See pr #320

@sodabrew sodabrew added this to the 3.0.2 milestone Feb 9, 2017
@sodabrew sodabrew mentioned this pull request Feb 9, 2017
carlwgeorge pushed a commit to iuscommunity-pkg/php70u-pecl-memcached that referenced this pull request Feb 9, 2017
switch to pecl sources
enable test suite
open php-memcached-dev/php-memcached#319
  fix test suite for 32bits build
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.

None yet

3 participants