This repository was archived by the owner on Feb 1, 2022. It is now read-only.
PHP-1510: Fix tests for legacy opcode rerouting #877
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| @@ -34,7 +34,7 @@ for ($i = 0; $i < 10; ++$i) { | |||
| array('x' => $i, 'y' => str_repeat('a', 4*1024*1024)), | |||
| array('w' => 0) | |||
| ); | |||
| assertFalse($retval); | |||
| assertTrue($retval); | |||
There was a problem hiding this comment.
From the commit message:
Now that unacknowledged writes are routed through legacy opcodes, these write methods return true (as documented).
It appears that the false assertion was due to mongo_convert_write_api_return_to_legacy_retval() converting the response from an unacknowledged write command (likely an empty array) to a boolean.
|
LGTM, when "silent" is changed to something akin to "show_warnings" |
The extra is_gle_op() call was causing duplicate "safe" deprecation notices and MongoLog output in some tests. Existing code paths for write commands and legacy ops already include necessary output, so allow is_gle_op() to be silent when invoked for the purpose of routing unacknowledged writes to legacy opcodes.
Now that unacknowledged writes are routed through legacy opcodes, these write methods return true (as documented). It appears that the false assertion was due to mongo_convert_write_api_return_to_legacy_retval() converting the response from an unacknowledged write command (likely an empty array) to a boolean.
Stream notifications cannot be used to assert the write concern of unacknowledged writes, since they are routed through legacy opcodes instead of write commands. Use MongoLog to assert that no GLE is issued for these writes.
jmikola
force-pushed
the
1.6-legacy-opcodes
branch
from
7b84658 to
16c9a1e
Compare
Removed the negation in the |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
My mistake for not catching these test failures in #876. Please see full commit messages for an explanation of changes.
While sorting these out, I also noticed an irregularity with
append_getlasterror()and opened PHPC-1511. I don't believe this affects anything, since the function would not have been called ifis_gle_op()returned false, but the previous logic looked incorrect nevertheless.Note: This was opened against the wrong branch. Will be merged to
v1.6.