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

Issue#84 TP4, RSC15a (test), RSC19e (test), .. #87

Merged
merged 15 commits into from
Mar 3, 2017
Merged

Issue#84 TP4, RSC15a (test), RSC19e (test), .. #87

merged 15 commits into from
Mar 3, 2017

Conversation

jdavid
Copy link
Contributor

@jdavid jdavid commented Feb 23, 2017

No description provided.

Copy link
Member

@mattheworiordan mattheworiordan left a comment

Choose a reason for hiding this comment

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

One comment, otherwise looks good. Please confirm which of the points at #84 this addresses for clarity

]

for aux in fallback_hosts:
ably = AblyRest(token='foo', fallback_hosts=fallback_hosts)
Copy link
Member

Choose a reason for hiding this comment

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

Bit confused, you're passing in fallback_hosts yet that's a two dimensional array?

@jdavid jdavid mentioned this pull request Feb 23, 2017
@jdavid
Copy link
Contributor Author

jdavid commented Feb 23, 2017

Good catch, fixed (it's RSC15a).

self.assertGreater(token.expires, time.time()*1000)
self.assertIsNot(new_token, token)
# Second does not
with self.assertRaises(AblyAuthException):
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure this is correct if it fails? What is the value of TOKEN_EXPIRY_BUFFER? If it's say 2s, this test is very racy as the realtime system will still respond if the token is valid i.e. there is no buffer factored in and the TTL is literal.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TOKEN_EXPIRY_BUFFER is 15s, I think it is specified somewhere

@jdavid
Copy link
Contributor Author

jdavid commented Feb 28, 2017

Travis is complaining. The timeout errors I had them in local too with the sandbox server, they went away using the staging server.
The failures are bizarre, I don't get them here, I will rerun travis.

@mattheworiordan
Copy link
Member

Thanks @jdavid, looks good. 👍
Would be good to understand the failures though :/

@jdavid
Copy link
Contributor Author

jdavid commented Feb 28, 2017

Umhh, the last commit is to use staging, to see whether the bizarre errors go away or not (just temporary of course).
But Travis is not rebuilding, don't know why. @mattheworiordan could you trigger the rebuild manually ? (I don't have the rights to it myself).

@mattheworiordan
Copy link
Member

But Travis is not rebuilding, don't know why

It sometimes builds up a backlog we've found, seems to have run and failed :( Shout if you want to discuss the failures to see if we can help.

@jdavid
Copy link
Contributor Author

jdavid commented Mar 1, 2017

The builds are broken actually, it looks like Travis was victim of the Amazon issues. https://www.traviscistatus.com/incidents/hmwq9yy5dh9d

@mattheworiordan
Copy link
Member

Ok, restarted the builds now

@jdavid
Copy link
Contributor Author

jdavid commented Mar 2, 2017

Latest try, only the usual random time failures. The bizarre failure is gone. Locally I don't have the time failures, nor any other error. It looks to me that these are somehow related to Travis. There are the same kind of failures
in the master branch, e.g. https://travis-ci.org/ably/ably-python/jobs/203778174 so I think this should not block the PR to be merged.

@mattheworiordan
Copy link
Member

I agree, merging now.

Can you please do me a favour though and just raise an issue to address the intermittent test issues i.e. simply document which ones are intermittent so that we can pick this up at some point in the future?

@jdavid
Copy link
Contributor Author

jdavid commented Mar 3, 2017

#88

mattheworiordan and others added 15 commits March 3, 2017 13:10
* Use of py.test instead of nosetests
  * Added several plugins commented for the future
  * Use xdist to run tests in parallel, decreased time to 66 secs from 137
* Setup of tox flake8 environment for code standard checks
  * Ignored a lot of errors/warnings to kickstart this. TODO: Remove them from setup.cfg
* Coveralls moved to travis, as coverage should only be submited from CI servers
* Deleted test.py file (no idea what it was doing there)
Those two python versions are known to be troublesome and defective.
client_id is special as cannot be changed once it is set
Useful if run locally with a virtualenv inside the project, so pytest
does not run tests for other software.
@mattheworiordan mattheworiordan merged commit a546217 into ably:1-0-master Mar 3, 2017
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