diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 0455b61bd..97751b32e 100644 --- a/CONTRIBUTING.md +++ b/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 diff --git a/test_elasticsearch/test_async/test_server/test_clients.py b/test_elasticsearch/test_async/test_server/test_clients.py index ba322c796..301915d33 100644 --- a/test_elasticsearch/test_async/test_server/test_clients.py +++ b/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 diff --git a/test_elasticsearch/test_client/__init__.py b/test_elasticsearch/test_client/__init__.py index 82834986c..2a87d183f 100644 --- a/test_elasticsearch/test_client/__init__.py +++ b/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 "" == repr(self.client) - - def test_repr_subclass(self): - class OtherElasticsearch(Elasticsearch): - pass - - assert "" == 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") diff --git a/test_elasticsearch/test_cases.py b/test_elasticsearch/test_client/common.py similarity index 100% rename from test_elasticsearch/test_cases.py rename to test_elasticsearch/test_client/common.py diff --git a/test_elasticsearch/test_client/test_client.py b/test_elasticsearch/test_client/test_client.py new file mode 100644 index 000000000..0b264a4c4 --- /dev/null +++ b/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 "" == repr(self.client) + + def test_repr_subclass(self): + class OtherElasticsearch(Elasticsearch): + pass + + assert "" == 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) diff --git a/test_elasticsearch/test_client/test_cluster.py b/test_elasticsearch/test_client/test_cluster.py index a623f03ea..04bfa39b1 100644 --- a/test_elasticsearch/test_client/test_cluster.py +++ b/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) diff --git a/test_elasticsearch/test_client/test_indices.py b/test_elasticsearch/test_client/test_indices.py index 2d71593bd..9aef89f8c 100644 --- a/test_elasticsearch/test_client/test_indices.py +++ b/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) diff --git a/test_elasticsearch/test_client/test_overrides.py b/test_elasticsearch/test_client/test_overrides.py index bc3ea2ef3..63186df40 100644 --- a/test_elasticsearch/test_client/test_overrides.py +++ b/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) diff --git a/test_elasticsearch/test_client/test_utils.py b/test_elasticsearch/test_client/test_utils.py index 5b3ada1d2..a764d8d2f 100644 --- a/test_elasticsearch/test_client/test_utils.py +++ b/test_elasticsearch/test_client/test_utils.py @@ -20,15 +20,52 @@ import pytest +from elasticsearch.client import _normalize_hosts from elasticsearch.client.utils import _bulk_body, _escape, _make_path, query_params +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 TestQueryParams: - def setup_method(self, _): - self.calls = [] + calls = [] @query_params("simple_param") def func_to_wrap(self, *args, **kwargs): + self.calls = [] self.calls.append((args, kwargs)) def test_handles_params(self): @@ -102,54 +139,51 @@ def test_per_call_authentication(self): ) # If both are given values an error is raised. - with pytest.raises(ValueError) as e: + with pytest.raises( + ValueError, + match="Only one of 'http_auth' and 'api_key' may be passed at a time", + ): self.func_to_wrap(http_auth="key", api_key=("1", "2")) - assert ( - str(e.value) - == "Only one of 'http_auth' and 'api_key' may be passed at a time" - ) class TestMakePath: - def test_handles_unicode(self): - id = "中文" + id = "中文" + + @pytest.mark.parametrize("id", [id, id.encode("utf-8")]) + def test_handles_unicode(self, id): assert "/some-index/type/%E4%B8%AD%E6%96%87" == _make_path( "some-index", "type", id ) class TestEscape: - def test_handles_ascii(self): - string = "abc123" - assert b"abc123" == _escape(string) - - def test_handles_unicode(self): - string = "中文" - assert b"\xe4\xb8\xad\xe6\x96\x87" == _escape(string) - - def test_handles_bytestring(self): - string = b"celery-task-meta-c4f1201f-eb7b-41d5-9318-a75a8cfbdaa0" - assert string == _escape(string) + @pytest.mark.parametrize( + ["left", "right"], + [ + ("abc123", b"abc123"), + ("中文", b"\xe4\xb8\xad\xe6\x96\x87"), + ( + b"celery-task-meta-c4f1201f-eb7b-41d5-9318-a75a8cfbdaa0", + b"celery-task-meta-c4f1201f-eb7b-41d5-9318-a75a8cfbdaa0", + ), + ], + ) + def test_handles_ascii_unicode_bytestring(self, left, right): + assert right == _escape(left) class TestBulkBody: - def test_proper_bulk_body_as_string_is_not_modified(self): - string_body = '"{"index":{ "_index" : "test"}}\n{"field1": "value1"}"\n' - assert string_body == _bulk_body(None, string_body) - - def test_proper_bulk_body_as_bytestring_is_not_modified(self): - bytestring_body = b'"{"index":{ "_index" : "test"}}\n{"field1": "value1"}"\n' - assert bytestring_body == _bulk_body(None, bytestring_body) - - def test_bulk_body_as_string_adds_trailing_newline(self): - string_body = '"{"index":{ "_index" : "test"}}\n{"field1": "value1"}"' - assert '"{"index":{ "_index" : "test"}}\n{"field1": "value1"}"\n' == _bulk_body( - None, string_body - ) - - def test_bulk_body_as_bytestring_adds_trailing_newline(self): - bytestring_body = b'"{"index":{ "_index" : "test"}}\n{"field1": "value1"}"' - assert ( - b'"{"index":{ "_index" : "test"}}\n{"field1": "value1"}"\n' - == _bulk_body(None, bytestring_body) - ) + id = '"{"index":{ "_index" : "test"}}\n{"field1": "value1"}"' + id_newline = id + "\n" + + @pytest.mark.parametrize( + ["left", "right"], + [ + (id_newline, id_newline), + (id_newline.encode("utf-8"), id_newline.encode("utf-8")), + (id_newline, id), + (id_newline.encode("utf-8"), id.encode("utf-8")), + ], + ) + def test_proper_bulk_string_bytestring(self, left, right): + assert left == _bulk_body(None, right) diff --git a/test_elasticsearch/test_server/test_clients.py b/test_elasticsearch/test_server/test_clients.py index facafe752..655e7eb13 100644 --- a/test_elasticsearch/test_server/test_clients.py +++ b/test_elasticsearch/test_server/test_clients.py @@ -18,6 +18,8 @@ from __future__ import unicode_literals +import pytest + def test_indices_analyze_unicode(sync_client): resp = sync_client.indices.analyze(body='{"text": "привет"}') @@ -34,33 +36,25 @@ def test_indices_analyze_unicode(sync_client): } -def test_bulk_works_with_string_body(sync_client): +class TestBulk: docs = '{ "index" : { "_index" : "bulk_test_index", "_id" : "1" } }\n{"answer": 42}' - resp = sync_client.bulk(body=docs) - - assert resp["errors"] is False - assert 1 == len(resp["items"]) - - -def test_bulk_works_with_bytestring_body(sync_client): - docs = ( - b'{ "index" : { "_index" : "bulk_test_index", "_id" : "2" } }\n{"answer": 42}' - ) - resp = sync_client.bulk(body=docs) - assert resp["errors"] is False - assert 1 == len(resp["items"]) - - # Pop inconsistent items before asserting - resp["items"][0]["index"].pop("_id") - resp["items"][0]["index"].pop("_version") - assert resp["items"][0] == { - "index": { - "_index": "bulk_test_index", - "result": "created", - "_shards": {"total": 2, "successful": 1, "failed": 0}, - "_seq_no": 0, - "_primary_term": 1, - "status": 201, + @pytest.mark.parametrize("docs", [docs, docs.encode("utf-8")]) + def test_bulk_works_with_string_bytestring_body(self, docs, sync_client): + response = sync_client.bulk(body=docs) + + assert (response["errors"]) is False + assert len(response["items"]) == 1 + # Pop inconsistent items before asserting + response["items"][0]["index"].pop("_id") + response["items"][0]["index"].pop("_version") + assert response["items"][0] == { + "index": { + "_index": "bulk_test_index", + "result": "created", + "_shards": {"total": 2, "successful": 1, "failed": 0}, + "_seq_no": 0, + "_primary_term": 1, + "status": 201, + } } - }