PHPLIB-918: Add test that reads are not retried in a transaction #963
Conversation
jmikola
changed the title
There was a problem hiding this comment.
For consistency in the future, please use PHPLIB-###: as a commit prefix instead of wrapping the issue in brackets. I revised the PR title, which should be sufficient to ensure the correct message is used when squash-merging.
The commit message is missing a comment indicating the mongodb/specifications commit to which you've synced. When we discussed this earlier, I referenced the commits in #961 as an example. Please add the following to your commit message:
Synced with mongodb/specifications@<full commit hash>
GitHub will then render that as a short link in its UI, as in f8eff8d.
Feel free to amend your commit and force-push to your PR branch.
| "tests": [ | ||
| { | ||
| "description": "AbortTransaction succeeds after handshake network error", | ||
| "skipReason": "DRIVERS-2032: Pinned servers need to be checked if they are still selectable", |
There was a problem hiding this comment.
I noted from the downstream changes in DRIVERS-746 that both of the "retryable" tests are skipped, so it was not necessary to add these to UnifiedSpecTest::$incompleteTests.
There was a problem hiding this comment.
I noted it too and that is why I didn't add new lines in UnifiedSpecTest::$incompleteTests. Also I double checked the DRIVERS-2032 ticket which is the parent ticket of DRIVERS-746.
levon80999
force-pushed
the
PHPLIB-918
branch
from
c1da0a5 to
1a0d4e0
Compare
levon80999
force-pushed
the
PHPLIB-918
branch
from
1a0d4e0 to
03735ac
Compare
There was a problem hiding this comment.
CI failures are unrelated. I believe they are actually both fixed in master, but your PR branch is based on an older master commit. For example, the tests/UnifiedSpecTests/change-streams directory corresponding to this PR's commit doesn't show any recent modifications to the two failing change stream tests, which were recently updated in a292d07 and f8eff8d. For future reference, I suggest rebasing your PR branch on master regularly if you're already comfortable with force-pushing. It will limit conflicts (granted, that's not an issue here) and help avoid unrelated CI failures that might have been recently addressed.
Please just remove the full issue URL from the commit message when squash-merging, as I mentioned in #967 (comment).
|
@levon80999: FYI, the merge commit (4bc4607) was missing the following: |
…godb#963) Co-authored-by: levon80999 <levonb@ucraft.com>
https://jira.mongodb.org/browse/PHPLIB-918
Synced with mongodb/specifications@c5d1c77