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

Rewind array after foreach by reference in DataCollectionTest #265

Merged
merged 1 commit into from
Nov 24, 2014
Merged

Rewind array after foreach by reference in DataCollectionTest #265

merged 1 commit into from
Nov 24, 2014

Conversation

SiebelsTim
Copy link
Contributor

HHVM's behaviour on foreach by reference might differ from php. This is happening here.
HHVM will incremend the internal array pointer while php doesn't. This causes test to fail on hhvm

See https://github.com/facebook/hhvm/blob/master/hphp/doc/inconsistencies#L29-L35

This increases klein on HHVM from 97.66% to 100% o/

HHVM's behaviour on foreach by reference might differ from php. This is happening here.
HHVM will incremend the internal array pointer while php doesn't. This causes test to fail on hhvm

See https://github.com/facebook/hhvm/blob/master/hphp/doc/inconsistencies#L29-L35

This increases klein on HHVM from 97.66% to 100% o/
@SiebelsTim
Copy link
Contributor Author

At least for the commit http://hhvm.com/frameworks/ tests against.

@Rican7 Rican7 self-assigned this Nov 24, 2014
@Rican7 Rican7 added this to the 2.2 milestone Nov 24, 2014
@Rican7
Copy link
Member

Rican7 commented Nov 24, 2014

Thank you so much for this PR!
I really appreciate the help in getting Klein to 100%!

Merging this right away!

Rican7 added a commit that referenced this pull request Nov 24, 2014
Rewind array after foreach by reference in DataCollectionTest

Thanks to @SiebelsTim we should now have 100% test compatibility with HHVM!
@Rican7 Rican7 merged commit 744c07b into klein:master Nov 24, 2014
@SiebelsTim SiebelsTim deleted the fix-hhvm-foreachbyref branch November 24, 2014 17:08
@Rican7 Rican7 modified the milestones: 2.1, 2.2 Nov 1, 2016
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.

2 participants