Conversation
Otherwise the last assert in the test is not done.
|
ping |
paddybyers
left a comment
paddybyers
left a comment
There was a problem hiding this comment.
I think it would be best to create a new integration branch for the push functionality, and raise pull requests against that. That means we can merge individual PRs to that branch, and can merge finally to master once there is a coherent set of functionality implemented.
tests/PushAdmin.php
Outdated
| * RSH1a | ||
| */ | ||
| public function testAdminPublish() { | ||
| $recipient = [ 'clientId' => 'ablyChannel' ]; |
There was a problem hiding this comment.
Does this work? It's supposed to be a push recipient with a transportType field of type DevicePushTransportType. In order to cause a push message to be sent to an Ably channel for testing, then the transportType must be ablyChannel. Then the other recipient details must include channel, ablyKey and ablyUrl. See for example https://github.com/ably/ably-ios/blob/develop/Spec/PushAdmin.swift#L232
There was a problem hiding this comment.
The server is not rejecting the message.
This is also like in the curl example at https://docs.ably.io/rest-api/#push-publish where only the clientId is given:
...
"recipient": {
"clientId": "myClientId"
},
...And the documentation at https://docs.ably.io/client-lib-development-guide/features/#RSH1a doesn't say anything about transportType etc., just:
... using the special test-only ablyChannel recipient ...
There was a problem hiding this comment.
The recipient needs to look like:
transportType: 'ablyChannel',
channel: <channel on which to publish the message>,
ablyKey: <Ably API key string>,
ablyUrl: <Ably endpoint URL>
In that case, when you post the message to be published, it will be delivered to an Ably channel and you'll be able to retrieve it using history.
There was a problem hiding this comment.
Done but history doesn't work, test failure.
There was a problem hiding this comment.
@jdavid what type of failure? Do you have a log? Are you using a channel with history enabled?
There was a problem hiding this comment.
The failure is Failed asserting that 0 matches expected 1. ; the test pushes to pushenabled:push_admin_publish-ok see https://github.com/ably/ably-php/blob/RSH1a/tests/PushAdminTest.php#L29
Then gets the channel history. I expect to be 1 item in the history, but there're zero items.
I tried locally adding "persisted": true to the namespace but got the same failure.
I've added debugging, search for testAdminPublish in https://api.travis-ci.org/v3/job/455908053/log.txt
There was a problem hiding this comment.
@jdavid have reminded @paddybyers to look at this...
There was a problem hiding this comment.
@jdavid I've had a look at the code and I can't see why it wouldn't work. I'm assuming that the value of $testApp->server is a URL, ie https://sandbox-rest.ably.io.
The channel in use is a pushenabled channel whereas it doesn't need to be push-enabled for this test to work. Strictly, since you're getting history on the channel you should use a persisted:xx channel for the test, but I'm sure you're getting history immediately after the call so any channel should work.
So I suggest that either (1) you help me with getting a local setup to help me reproduce it, or (2) you try at a time that I can look at the logs and see what's going on.
There was a problem hiding this comment.
Well, I've just re-run the tests and now it's all green.. Maybe something changed in the server in these 2.5 months?
Can we say it's good for merging now? Approved?
|
Created a new branch named |
This reverts commit 7899935.
3 participants
RSH1a New push.admin.publish