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

Don't assert on error messages. #925

Merged
merged 1 commit into from
Dec 8, 2019
Merged

Don't assert on error messages. #925

merged 1 commit into from
Dec 8, 2019

Conversation

tcard
Copy link
Contributor

@tcard tcard commented Dec 5, 2019

They may change (as this one has). That's what codes are for.

They may change (as this one has). That's what codes are for.
@tcard tcard requested a review from QuintinWillison December 5, 2019 13:38
@tcard tcard self-assigned this Dec 5, 2019
@tcard
Copy link
Contributor Author

tcard commented Dec 5, 2019

While the build still broken, the failures that this PR intended to fix does seem to have been fixed:

    ✓ JWT_and_realtime__client_initialized_with_a_JWT_token_in_ClientOptions__with_valid_credentials__pulls_stats_successfully (0.706 seconds)
    ✓ JWT_and_realtime__client_initialized_with_a_JWT_token_in_ClientOptions__with_invalid_credentials__fails_to_connect_with_reason__invalid_signature_ (0.115 seconds)
    ✓ JWT_and_realtime__when_using_authUrl__with_valid_credentials__fetches_a_channels_and_posts_a_message (0.776 seconds)
    ✓ JWT_and_realtime__when_using_authUrl__with_wrong_credentials__fails_to_connect_with_reason__invalid_signature_ (0.584 seconds)
    ✓ JWT_and_realtime__when_using_authUrl__when_token_expires__receives_a_40142_error_from_the_server (5.353 seconds)
    ✓ JWT_and_realtime__when_using_authUrl__when_the_server_sends_and_AUTH_protocol_message__client_reauths_correctly_without_going_through_a_disconnection (5.122 seconds)
    ✓ JWT_and_realtime__when_using_authCallback__with_valid_credentials__pulls_stats_successfully (1.130 seconds)
    ✓ JWT_and_realtime__when_using_authCallback__with_invalid_credentials__fails_to_connect (0.603 seconds)
    ✓ JWT_and_realtime__when_token_expires_and_has_a_means_to_renew__reconnects_using_authCallback_and_obtains_a_new_token (3.731 seconds)
    ✓ JWT_and_realtime__when_the_token_request_includes_a_clientId__the_clientId_is_the_same_specified_in_the_JWT_token_request (0.636 seconds)
    ✓ JWT_and_realtime__when_the_token_request_includes_subscribe_only_capabilities__fails_to_publish_to_a_channel_with_subscribe_only_capability (0.650 seconds)

The other failing tests seem to be inconsistently failing: they do pass in my machine, and they aren't even the same in the two build jobs that were run: #5326.1 and #5326.2. I think it's a safe bet to say they're unrelated to this PR.

So I think this should be merged.

@tcard tcard merged commit a89a864 into develop Dec 8, 2019
@tcard tcard deleted the jwt-signature-fail-code branch December 8, 2019 08:12
maratal pushed a commit that referenced this pull request Jul 19, 2023
[FEA-547] Change jsbin domain from ably.io to ably.com (AUGUST)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants