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.

Sign up for GitHub

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

Jump to bottom

Drop the ultra-flaky test for orphaned subscriptions. #2215

Merged
merged 1 commit into from
Sep 2, 2016
Merged

Drop the ultra-flaky test for orphaned subscriptions. #2215

merged 1 commit into from
Sep 2, 2016

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Aug 28, 2016

See:
#2080 (comment)

Closes #2080.

@tseaver tseaver added api: pubsub Issues related to the Pub/Sub API. flaky labels Aug 28, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 28, 2016
@daspecster
Copy link
Contributor

In #2080 (comment), you mentioned there are other tests that cover this case?

Otherwise LGTM. I agree we need to get the build green with as few logical gaps in test coverage as possible.

I wonder if we should have some kind of secondary set of system tests? It might be nice to track these kinds of issues for the upstream guys?

@tseaver
Copy link
Contributor Author

tseaver commented Aug 29, 2016

@daspecster Another option would be to mark the test with @unittest.expectedFailure.

@daspecster daspecster mentioned this pull request Aug 29, 2016
Merged
@dhermes
Copy link
Contributor

dhermes commented Aug 29, 2016

See my comment on #2080. Let's hold off on merging this until we can get @tmatsuo to weigh in (or until a fixed time interval passes not hearing from him)

@tseaver
Copy link
Contributor Author

tseaver commented Aug 29, 2016

@dhermes WDYT of leaving it in place, but decorating it with @unittest.expected_failure? The back-end folks could use to to investigate, and meanwhile we would be green.

@dhermes
Copy link
Contributor

dhermes commented Aug 29, 2016

Seems mostly OK. But an issue needs to be filed to follow up and fix it.

@tseaver
Copy link
Contributor Author

tseaver commented Aug 29, 2016

#2080 would stay open if we followed that route: ideally, we would assign it to @tmatsuo.

@dhermes
Copy link
Contributor

dhermes commented Aug 29, 2016

OK

@tseaver
Mark ultra-flaky 'orphan subscrpition' test as an expected failure.
27eec9d 
Allows our tests to go green while the back-end investigates.

Also, drop the ultra-flaky 'self.assertFalse(topic.exists())' in
'test_list_subscriptions'.  It is redundant (names are unique), and dropping
it lets the test pass normally.

Toward #2080, #2111.
@tseaver
Copy link
Contributor Author

tseaver commented Aug 29, 2016

I just backed out the "delete the test" change and marked it with unittest.expectedFailure.

I also dropped the self.assertFalse(topic.exists()) (see #2111).

dhermes
dhermes reviewed Aug 30, 2016
View reviewed changes
system_tests/pubsub.py
@@ -135,7 +135,6 @@ def test_create_subscription_w_ack_deadline(self):
def test_list_subscriptions(self):
TOPIC_NAME = 'list-sub' + unique_resource_id('-')
topic = Config.CLIENT.topic(TOPIC_NAME)
self.assertFalse(retry_unavailable(topic.exists)())

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor

dhermes commented Sep 1, 2016

Go ahead and merge

@tseaver tseaver merged commit efe359a into googleapis:master Sep 2, 2016
@tseaver tseaver deleted the 2080-pubsub-drop-flaky-orphan-test branch September 2, 2016 12:50
@tseaver tseaver mentioned this pull request Sep 6, 2016
Closed
@dhermes dhermes mentioned this pull request Sep 19, 2016
Merged
parthea pushed a commit that referenced this pull request Sep 22, 2023
@dizcology @danoscarmike
update streaming samples [(#2215)](GoogleCloudPlatform/python-docs-sa…
b686c70 
…mples#2215)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
api: pubsub Issues related to the Pub/Sub API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@tseaver @daspecster @dhermes @googlebot