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

[rackspace] fixing broken tests #1918

Merged
merged 10 commits into from
Jul 1, 2013
Merged

[rackspace] fixing broken tests #1918

merged 10 commits into from
Jul 1, 2013

Conversation

krames
Copy link
Member

@krames krames commented Jun 26, 2013

This pull request address several issues that occur when run against the live Rackspace Cloud.

I have also increased the default fog timeout to 2000 seconds when running tests against a live cloud to address the brittleness of the Rackspace tests. I am receptive to hearing everyone's thoughts on this change.



unless Fog.mocking?
Fog.timeout = 2000
Copy link
Member Author

Choose a reason for hiding this comment

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

@geemus I don't know if this is the best place to make this change or if this is even a change we want to make. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Good question. It might point to us needing a longer default timeout in general (ie most people are running live stuff, so if the timeout is only good for mock it isn't very good at all).

Copy link
Member Author

Choose a reason for hiding this comment

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

I think your right. I know I have seen this issue popup in knife-rackspace more times than it should have.

What do you think about increasing the default timeout in fog to 1000 (~16 minutes)? I would still like to keep the testing timeout to 2000. Do you think I should relocate this line to fog test helper file?

Copy link
Member

Choose a reason for hiding this comment

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

Any reason why the testing one would be expected to be twice as long? I'm ok with increasing the default if we think that needs to occur. I guess it might hint that a per-provider default might be worth pursuing (as I haven't heard about this issue from other providers, which of course doesn't mean it isn't happening).

Copy link
Member Author

Choose a reason for hiding this comment

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

I wanted to give the testing environment more than enough time to complete an operation without having to worry about getting a timeout exception. Outside of testing, I can foresee a developer wanting to get a timeout sooner than 2000 (33 minutes) and having the code to handle it accordingly. Maybe I am wrong?

Copy link
Member

Choose a reason for hiding this comment

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

Probably fair. I had just wondered why this might be exceptional compared to real usage (since ideally the tests mimic real usage to some extent). Makes sense though.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 945319d on rackspace:fix_tests into 8c87f82 on fog:master.

krames pushed a commit that referenced this pull request Jul 1, 2013
[rackspace] fixing broken tests
@krames krames merged commit fb60a31 into fog:master Jul 1, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants