-
Notifications
You must be signed in to change notification settings - Fork 37
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
Update cluster list tests #1418
base: main
Are you sure you want to change the base?
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
8b8c8a0
to
ec26ed2
Compare
assert len(running_clusters) > 0 | ||
assert len(running_clusters) == len( | ||
all_clusters | ||
all_clusters = Cluster.list(show_all=True).get("den_clusters", {}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that this we should not remove test_cluster_list_pythonic
(maybe just re-name it if necessary), because it tests the default behaviour of Cluster.list()
.
Seems to me we are going to show by default only part of the running clusters by default (based on @carolineechen comments on Josh's PR), so IMO we need to have two separate tests:
- Default behavior of Cluster.list()
- Tests the behaviour of Cluster.list(show_all=true)
wdyt?
@@ -1035,7 +1026,7 @@ def test_cluster_list_pythonic(self, cluster): | |||
|
|||
@pytest.mark.level("local") | |||
@pytest.mark.clustertest | |||
def test_cluster_list_all_pythonic(self, cluster): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to the note above, not sure that we need to delete this test
18ed652
to
ed562d5
Compare
No description provided.