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

RTN14b #167

Merged
merged 3 commits into from
Jan 29, 2016
Merged

RTN14b #167

merged 3 commits into from
Jan 29, 2016

Conversation

ricardopereira
Copy link
Contributor

The test doesn't pass.

@ricardopereira
Copy link
Contributor Author

The test is not passing. The lib only gives the 40142 - token expired and it doesn't retry to request a new token. Need fix.

@mattheworiordan
Copy link
Member

I still don't understand why one of those tests passes and the other fails when both seem to not have any mechanism to renew the token. The tests look wrong to me

@ricardopereira ricardopereira force-pushed the RTN14b branch 2 times, most recently from 5a5da32 to 7a3eff3 Compare January 29, 2016 08:18
// New token
expect(client.auth().tokenDetails!.token).toNot(equal(options.token))
done()
case .Failed:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind adding the Disconnected state in here too as it should not reach Disconnected, Suspended or Failed states?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@mattheworiordan
Copy link
Member

Few small comments, please address those and from my side I am happy for you to merge then

@ricardopereira
Copy link
Contributor Author

All comments addressed.
@tcard PTAL

@tcard
Copy link
Contributor

tcard commented Jan 29, 2016

LGTM

tcard added a commit that referenced this pull request Jan 29, 2016
@tcard tcard merged commit 858d7a1 into master Jan 29, 2016
@ricardopereira ricardopereira deleted the RTN14b branch January 30, 2016 07:59
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.

3 participants