Refactor Tests for test_client directory

CONTRIBUTING.md

@@ -50,7 +50,7 @@ The process for contributing to any of the Elasticsearch repositories is similar
 
 2. Run the linter and test suite to ensure your changes do not break existing code:
 
-   ```
+   ```bash
    # Install Nox for task management
    $ python -m pip install nox
 

test_elasticsearch/test_async/test_server/test_clients.py

@@ -29,15 +29,10 @@ async def test_indices_analyze(self, async_client):
 
 
 class TestBulk:
-    async def test_bulk_works_with_string_body(self, async_client):
-        docs = '{ "index" : { "_index" : "bulk_test_index", "_id" : "1" } }\n{"answer": 42}'
-        response = await async_client.bulk(body=docs)
-
-        assert response["errors"] is False
-        assert len(response["items"]) == 1
+    docs = '{ "index" : { "_index" : "bulk_test_index", "_id" : "1" } }\n{"answer": 42}'
 
-    async def test_bulk_works_with_bytestring_body(self, async_client):
-        docs = b'{ "index" : { "_index" : "bulk_test_index", "_id" : "2" } }\n{"answer": 42}'
+    @pytest.mark.parametrize("docs", [docs, docs.encode("utf-8")])
+    async def test_bulk_works_with_string_bytestring_body(self, docs, async_client):
         response = await async_client.bulk(body=docs)
 
         assert response["errors"] is False

test_elasticsearch/test_client/__init__.py

@@ -14,109 +14,3 @@
 #  KIND, either express or implied.  See the License for the
 #  specific language governing permissions and limitations
 #  under the License.
-
-from __future__ import unicode_literals
-
-from elasticsearch.client import Elasticsearch, _normalize_hosts
-
-from ..test_cases import DummyTransportTestCase
-
-
-class TestNormalizeHosts:
-    def test_none_uses_defaults(self):
-        assert [{}] == _normalize_hosts(None)
-
-    def test_strings_are_used_as_hostnames(self):
-        assert [{"host": "elastic.co"}] == _normalize_hosts(["elastic.co"])
-
-    def test_strings_are_parsed_for_port_and_user(self):
-        assert [
-            {"host": "elastic.co", "port": 42},
-            {"host": "elastic.co", "http_auth": "user:secre]"},
-        ] == _normalize_hosts(["elastic.co:42", "user:secre%5D@elastic.co"])
-
-    def test_strings_are_parsed_for_scheme(self):
-        assert [
-            {"host": "elastic.co", "port": 42, "use_ssl": True},
-            {
-                "host": "elastic.co",
-                "http_auth": "user:secret",
-                "use_ssl": True,
-                "port": 443,
-                "url_prefix": "/prefix",
-            },
-        ] == _normalize_hosts(
-            ["https://elastic.co:42", "https://user:secret@elastic.co/prefix"]
-        )
-
-    def test_dicts_are_left_unchanged(self):
-        assert [{"host": "local", "extra": 123}] == _normalize_hosts(
-            [{"host": "local", "extra": 123}]
-        )
-
-    def test_single_string_is_wrapped_in_list(self):
-        assert [{"host": "elastic.co"}] == _normalize_hosts("elastic.co")
-
-
-class TestClient(DummyTransportTestCase):
-    def test_request_timeout_is_passed_through_unescaped(self):
-        self.client.ping(request_timeout=0.1)
-        calls = self.assert_url_called("HEAD", "/")
-        assert [({"request_timeout": 0.1}, {}, None)] == calls
-
-    def test_params_is_copied_when(self):
-        rt = object()
-        params = dict(request_timeout=rt)
-        self.client.ping(params=params)
-        self.client.ping(params=params)
-        calls = self.assert_url_called("HEAD", "/", 2)
-        assert [
-            ({"request_timeout": rt}, {}, None),
-            ({"request_timeout": rt}, {}, None),
-        ] == calls
-        assert not (calls[0][0] is calls[1][0])
-
-    def test_headers_is_copied_when(self):
-        hv = "value"
-        headers = dict(Authentication=hv)
-        self.client.ping(headers=headers)
-        self.client.ping(headers=headers)
-        calls = self.assert_url_called("HEAD", "/", 2)
-        assert [
-            ({}, {"authentication": hv}, None),
-            ({}, {"authentication": hv}, None),
-        ] == calls
-        assert not (calls[0][0] is calls[1][0])
-
-    def test_from_in_search(self):
-        self.client.search(index="i", from_=10)
-        calls = self.assert_url_called("POST", "/i/_search")
-        assert [({"from": "10"}, {}, None)] == calls
-
-    def test_repr_contains_hosts(self):
-        assert "<Elasticsearch([{}])>" == repr(self.client)
-
-    def test_repr_subclass(self):
-        class OtherElasticsearch(Elasticsearch):
-            pass
-
-        assert "<OtherElasticsearch([{}])>" == repr(OtherElasticsearch())
-
-    def test_repr_contains_hosts_passed_in(self):
-        assert "es.org" in repr(Elasticsearch(["es.org:123"]))
-
-    def test_repr_truncates_host_to_5(self):
-        hosts = [{"host": "es" + str(i)} for i in range(10)]
-        es = Elasticsearch(hosts)
-        assert "es5" not in repr(es)
-        assert "..." in repr(es)
-
-    def test_index_uses_post_if_id_is_empty(self):
-        self.client.index(index="my-index", id="", body={})
-
-        self.assert_url_called("POST", "/my-index/_doc")
-
-    def test_index_uses_put_if_id_is_not_empty(self):
-        self.client.index(index="my-index", id=0, body={})
-
-        self.assert_url_called("PUT", "/my-index/_doc/0")

test_elasticsearch/test_client/test_client.py

@@ -0,0 +1,77 @@
+#  Licensed to Elasticsearch B.V. under one or more contributor
+#  license agreements. See the NOTICE file distributed with
+#  this work for additional information regarding copyright
+#  ownership. Elasticsearch B.V. licenses this file to you under
+#  the Apache License, Version 2.0 (the "License"); you may
+#  not use this file except in compliance with the License.
+#  You may obtain a copy of the License at
+#
+# 	http://www.apache.org/licenses/LICENSE-2.0
+#
+#  Unless required by applicable law or agreed to in writing,
+#  software distributed under the License is distributed on an
+#  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+#  KIND, either express or implied.  See the License for the
+#  specific language governing permissions and limitations
+#  under the License.
+
+
+import pytest
+
+from elasticsearch.client import Elasticsearch
+
+from .common import DummyTransportTestCase
+
+
+class TestClient(DummyTransportTestCase):
+    def test_request_timeout_is_passed_through_unescaped(self):
+        self.client.ping(request_timeout=0.1)
+        calls = self.assert_url_called("HEAD", "/")
+        assert [({"request_timeout": 0.1}, {}, None)] == calls
+
+    def test_params_is_copied_when(self):
+        params = {"request_timeout": object()}
+        self.client.ping(params=params)
+        self.client.ping(params=params)
+        calls = self.assert_url_called("HEAD", "/", 2)
+        assert [(params, {}, None), (params, {}, None)] == calls
+        assert calls[0][0] is not calls[1][0]
+
+    def test_headers_is_copied_when(self):
+        headers = {"authentication": "value"}
+        self.client.ping(headers=headers)
+        self.client.ping(headers=headers)
+        calls = self.assert_url_called("HEAD", "/", 2)
+        assert [({}, headers, None), ({}, headers, None)] == calls
+        assert calls[0][0] is not calls[1][0]
+
+    def test_from_in_search(self):
+        self.client.search(index="i", from_=10)
+        calls = self.assert_url_called("POST", "/i/_search")
+        assert [({"from": b"10"}, {}, None)] == calls
+
+    def test_repr_contains_hosts(self):
+        assert "<Elasticsearch([{}])>" == repr(self.client)
+
+    def test_repr_subclass(self):
+        class OtherElasticsearch(Elasticsearch):
+            pass
+
+        assert "<OtherElasticsearch([{}])>" == repr(OtherElasticsearch())
+
+    def test_repr_contains_hosts_passed_in(self):
+        assert "es.org" in repr(Elasticsearch(["es.org:123"]))
+
+    def test_repr_truncates_host_to_5(self):
+        hosts = [{"host": "es" + str(i)} for i in range(10)]
+        es = Elasticsearch(hosts)
+        assert '{"host": "es5"}' not in repr(es)
+        assert "..." in repr(es)
+
+    @pytest.mark.parametrize(
+        ["id", "request_method", "url"],
+        [("", "POST", "/my-index/_doc"), (0, "PUT", "/my-index/_doc/0")],
+    )
+    def test_index_uses_id(self, id, request_method, url):
+        self.client.index(index="my-index", id=id, body={})
+        self.assert_url_called(request_method, url)

test_elasticsearch/test_client/test_cluster.py

@@ -15,30 +15,34 @@
 #  specific language governing permissions and limitations
 #  under the License.
 
-from test_elasticsearch.test_cases import DummyTransportTestCase
+import pytest
 
+from .common import DummyTransportTestCase
 
-class TestCluster(DummyTransportTestCase):
-    def test_stats_without_node_id(self):
-        self.client.cluster.stats()
-        self.assert_url_called("GET", "/_cluster/stats")
-
-    def test_stats_with_node_id(self):
-        self.client.cluster.stats("node-1")
-        self.assert_url_called("GET", "/_cluster/stats/nodes/node-1")
-
-        self.client.cluster.stats(node_id="node-2")
-        self.assert_url_called("GET", "/_cluster/stats/nodes/node-2")
-
-    def test_state_with_index_without_metric_defaults_to_all(self):
-        self.client.cluster.state()
-        self.assert_url_called("GET", "/_cluster/state")
 
-        self.client.cluster.state(metric="cluster_name")
-        self.assert_url_called("GET", "/_cluster/state/cluster_name")
-
-        self.client.cluster.state(index="index-1")
-        self.assert_url_called("GET", "/_cluster/state/_all/index-1")
-
-        self.client.cluster.state(index="index-1", metric="cluster_name")
-        self.assert_url_called("GET", "/_cluster/state/cluster_name/index-1")
+class TestCluster(DummyTransportTestCase):
+    @pytest.mark.parametrize("node_id", [None, "node-1", "node-2"])
+    def test_stats_node_id(self, node_id):
+        self.client.cluster.stats(node_id=node_id)
+        url = "/_cluster/stats"
+        if node_id:
+            url += "/nodes/" + node_id
+        self.assert_url_called("GET", url)
+
+    @pytest.mark.parametrize(
+        ["index", "metric", "url_suffix"],
+        [
+            (None, None, None),
+            (None, "cluster_name", "/cluster_name"),
+            ("index-1", None, "/_all/index-1"),
+            ("index-1", "cluster_name", "/cluster_name/index-1"),
+        ],
+    )
+    def test_state_with_index_without_metric_defaults_to_all(
+        self, index, metric, url_suffix
+    ):
+        self.client.cluster.state(index=index, metric=metric)
+        url = "/_cluster/state"
+        if url_suffix:
+            url += url_suffix
+        self.assert_url_called("GET", url)

test_elasticsearch/test_client/test_indices.py

@@ -17,7 +17,7 @@
 
 import pytest
 
-from test_elasticsearch.test_cases import DummyTransportTestCase
+from .common import DummyTransportTestCase
 
 
 class TestIndices(DummyTransportTestCase):
@@ -33,10 +33,7 @@ def test_exists_index(self):
         self.client.indices.exists("second.index,third/index")
         self.assert_url_called("HEAD", "/second.index,third%2Findex")
 
-    def test_passing_empty_value_for_required_param_raises_exception(self):
+    @pytest.mark.parametrize("index", [None, [], ""])
+    def test_passing_empty_value_for_required_param_raises_exception(self, index):
         with pytest.raises(ValueError):
-            self.client.indices.exists(index=None)
-        with pytest.raises(ValueError):
-            self.client.indices.exists(index=[])
-        with pytest.raises(ValueError):
-            self.client.indices.exists(index="")
+            self.client.indices.exists(index=index)

test_elasticsearch/test_client/test_overrides.py

@@ -16,55 +16,40 @@
 #  specific language governing permissions and limitations
 #  under the License.
 
-from test_elasticsearch.test_cases import DummyTransportTestCase
+import pytest
 
+from .common import DummyTransportTestCase
 
-class TestOverriddenUrlTargets(DummyTransportTestCase):
-    def test_create(self):
-        self.client.create(index="test-index", id="test-id", body={})
-        self.assert_url_called("PUT", "/test-index/_create/test-id")
-
-        self.client.create(
-            index="test-index", doc_type="test-type", id="test-id", body={}
-        )
-        self.assert_url_called("PUT", "/test-index/test-type/test-id/_create")
-
-    def test_delete(self):
-        self.client.delete(index="test-index", id="test-id")
-        self.assert_url_called("DELETE", "/test-index/_doc/test-id")
-
-        self.client.delete(index="test-index", doc_type="test-type", id="test-id")
-        self.assert_url_called("DELETE", "/test-index/test-type/test-id")
-
-    def test_index(self):
-        self.client.index(index="test-index", body={})
-        self.assert_url_called("POST", "/test-index/_doc")
-
-        self.client.index(index="test-index", id="test-id", body={})
-        self.assert_url_called("PUT", "/test-index/_doc/test-id")
-
-    def test_update(self):
-        self.client.update(index="test-index", id="test-id", body={})
-        self.assert_url_called("POST", "/test-index/_update/test-id")
 
-        self.client.update(
-            index="test-index", doc_type="test-type", id="test-id", body={}
-        )
-        self.assert_url_called("POST", "/test-index/test-type/test-id/_update")
-
-    def test_cluster_state(self):
-        self.client.cluster.state()
-        self.assert_url_called("GET", "/_cluster/state")
-
-        self.client.cluster.state(index="test-index")
-        self.assert_url_called("GET", "/_cluster/state/_all/test-index")
-
-        self.client.cluster.state(index="test-index", metric="test-metric")
-        self.assert_url_called("GET", "/_cluster/state/test-metric/test-index")
-
-    def test_cluster_stats(self):
-        self.client.cluster.stats()
-        self.assert_url_called("GET", "/_cluster/stats")
-
-        self.client.cluster.stats(node_id="test-node")
-        self.assert_url_called("GET", "/_cluster/stats/nodes/test-node")
+class TestOverriddenUrlTargets(DummyTransportTestCase):
+    @pytest.mark.parametrize(
+        ["doc_type", "url_suffix"],
+        [(None, "/_create/test-id"), ("test-type", "/test-type/test-id/_create")],
+    )
+    def test_create(self, doc_type, url_suffix):
+        self.client.create(index="test-index", doc_type=doc_type, id="test-id", body={})
+        self.assert_url_called("PUT", "/test-index" + url_suffix)
+
+    @pytest.mark.parametrize(
+        ["doc_type", "url_suffix"],
+        [(None, "/_doc/test-id"), ("test-type", "/test-type/test-id")],
+    )
+    def test_delete(self, doc_type, url_suffix):
+        self.client.delete(index="test-index", doc_type=doc_type, id="test-id")
+        self.assert_url_called("DELETE", "/test-index" + url_suffix)
+
+    @pytest.mark.parametrize(
+        ["doc_type", "url_suffix"],
+        [(None, "/_update/test-id"), ("test-type", "/test-type/test-id/_update")],
+    )
+    def test_update(self, doc_type, url_suffix):
+        self.client.update(index="test-index", doc_type=doc_type, id="test-id", body={})
+        self.assert_url_called("POST", "/test-index" + url_suffix)
+
+    @pytest.mark.parametrize(
+        ["request_method", "id", "url_suffix"],
+        [("POST", None, ""), ("PUT", "test-id", "/test-id")],
+    )
+    def test_index(self, request_method, id, url_suffix):
+        self.client.index(index="test-index", id=id, body={})
+        self.assert_url_called(request_method, "/test-index/_doc" + url_suffix)