Skip to content

Added some tests around network partitioning, closing connections and …#819

Merged
beltran merged 1 commit into
masterfrom
python-simulacron-tests
Oct 5, 2017
Merged

Added some tests around network partitioning, closing connections and …#819
beltran merged 1 commit into
masterfrom
python-simulacron-tests

Conversation

@beltran
Copy link
Copy Markdown
Contributor

@beltran beltran commented Aug 29, 2017

…cluster._retry and idle connections with simulacron

@beltran beltran force-pushed the python-simulacron-tests branch 4 times, most recently from d2f5faf to 0f6f6b8 Compare September 5, 2017 15:02
@beltran beltran requested a review from mambocab September 6, 2017 18:03
Copy link
Copy Markdown
Contributor

@mambocab mambocab left a comment

Choose a reason for hiding this comment

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

nothing but nits here, except for a small question about putting things in tests vs tearDown.

# In this case HostConnection._replace shouldn't be called
self.assertNotIn("_replace", executor.called_functions)

clear_queries()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is handled in the tearDown, is this necessary? and if it's necessary for the assertion below, could you leave a comment saying so?


self.addCleanup(cluster.shutdown)

time_to_wait_for = 20
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I don't think this intermediate variable is necessary. I think the test docs make it clear what this sleep is for.

"""
Test to ensure that the connections aren't closed if there's an
OperationTimedOut in a normal query. This should only happen from the
HearBeatThread (in the case of a OperationTimedOut) with the default
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: I think we just want "heartbeat thread" here.

Comment thread tests/integration/simulacron/utils.py Outdated
"""
if then:
rows = None
column_types = None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This should probably do something more like

nose.tools.assert_is_none(rows)
nose.tools.assert_is_none(column_types)

-- we'd rather have failures than silently drop arguments IMO.

session = cluster.connect(wait_for_all_pools=True)
cluster.register_listener(listener)

number_of_queries = 10
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: same here as above -- is this intermediate variable necessary?

@beltran beltran force-pushed the python-simulacron-tests branch 6 times, most recently from 379f7f6 to 7bbbc38 Compare October 4, 2017 18:55
…cluster._retry and idle connections with simulacron
@beltran beltran force-pushed the python-simulacron-tests branch from 7bbbc38 to 5c77d93 Compare October 4, 2017 21:41
@beltran beltran merged commit b5cb3bf into master Oct 5, 2017
@beltran beltran deleted the python-simulacron-tests branch October 5, 2017 17:35
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.

2 participants