From a9b5351b8cc44823d337b4df6c491b00cff2078d Mon Sep 17 00:00:00 2001 From: piglei Date: Tue, 22 Oct 2024 18:09:42 +0800 Subject: [PATCH 1/4] refactor: improve code in test_network_config.py --- .../api/bkapp_model/test_network_config.py | 240 ++++++++---------- pyproject.toml | 3 +- 2 files changed, 105 insertions(+), 138 deletions(-) diff --git a/apiserver/paasng/tests/api/bkapp_model/test_network_config.py b/apiserver/paasng/tests/api/bkapp_model/test_network_config.py index b6b4bff183..ee42b6ba0c 100644 --- a/apiserver/paasng/tests/api/bkapp_model/test_network_config.py +++ b/apiserver/paasng/tests/api/bkapp_model/test_network_config.py @@ -16,185 +16,153 @@ # to the current version of the project delivered to anyone in the future. import pytest +from django.urls import reverse -from paasng.platform.bkapp_model.models import DomainResolution, SvcDiscConfig +from paasng.platform.applications.models import Application +from tests.utils.helpers import create_app pytestmark = pytest.mark.django_db(databases=["default", "workloads"]) +@pytest.fixture(autouse=True) +def bk_app2() -> Application: + """Create another application for testing""" + return create_app() + + class TestSvcDiscConfigViewSet: - @pytest.fixture() - def svc_disc(self, bk_app): - """创建一个 SvcDiscConfig 对象""" - return SvcDiscConfig.objects.create( - application=bk_app, - bk_saas=[ - { - "bkAppCode": "bk_app_code_test", - "moduleName": "module_name_test", - } - ], - ) + @pytest.fixture + def test_url(self, bk_app) -> str: + return reverse("api.applications.svc_disc", kwargs={"code": bk_app.code}) - def test_get_normal(self, api_client, bk_app, svc_disc): - url = f"/api/bkapps/applications/{bk_app.code}/svc_disc/" - response = api_client.get(url) - assert response.status_code == 200 - assert response.data["bk_saas"] == [{"bk_app_code": "bk_app_code_test", "module_name": "module_name_test"}] + @pytest.fixture + def with_default_disc(self, api_client, test_url, bk_app2): + """Create a default svc_disc object""" + resp = api_client.post(test_url, {"bk_saas": [{"bk_app_code": bk_app2.code, "module_name": "default"}]}) + return resp + + def test_get_missing(self, api_client, test_url): + response = api_client.get(test_url) - def test_get_missing(self, api_client, bk_app): - url = f"/api/bkapps/applications/{bk_app.code}/svc_disc/" - response = api_client.get(url) assert response.status_code == 404 - def test_upsert_normal(self, api_client, bk_app, svc_disc): - url = f"/api/bkapps/applications/{bk_app.code}/svc_disc/" - request_body = {"bk_saas": [{"bk_app_code": bk_app.code, "module_name": "default"}]} - response = api_client.post(url, request_body) + def test_get_normal(self, with_default_disc, api_client, bk_app2, test_url): + response = api_client.get(test_url) + + assert response.status_code == 200 + assert response.data["bk_saas"] == [{"bk_app_code": bk_app2.code, "module_name": "default"}] + + def test_upsert_normal(self, with_default_disc, api_client, bk_app, test_url): + response = api_client.post(test_url, {"bk_saas": [{"bk_app_code": bk_app.code, "module_name": "default"}]}) + assert response.status_code == 200 assert response.data["bk_saas"] == [{"bk_app_code": bk_app.code, "module_name": "default"}] - def test_upsert_module_absent(self, api_client, bk_app, svc_disc): - url = f"/api/bkapps/applications/{bk_app.code}/svc_disc/" - request_body = {"bk_saas": [{"bk_app_code": bk_app.code}]} - response = api_client.post(url, request_body) + def test_upsert_module_absent(self, api_client, bk_app, test_url): + response = api_client.post(test_url, {"bk_saas": [{"bk_app_code": bk_app.code}]}) + assert response.status_code == 200 assert response.data["bk_saas"] == [{"bk_app_code": bk_app.code, "module_name": None}] - def test_upsert_invalid_module(self, api_client, bk_app, svc_disc): - url = f"/api/bkapps/applications/{bk_app.code}/svc_disc/" - request_body = {"bk_saas": [{"bk_app_code": bk_app.code, "module_name": "test-invalid-module-name"}]} - response = api_client.post(url, request_body) + def test_upsert_invalid_module(self, api_client, bk_app, test_url): + response = api_client.post( + test_url, {"bk_saas": [{"bk_app_code": bk_app.code, "module_name": "test-invalid-module-name"}]} + ) + assert response.status_code == 400 - def test_upsert_duplicated_entries(self, api_client, bk_app, svc_disc): - url = f"/api/bkapps/applications/{bk_app.code}/svc_disc/" - request_body = { - "bk_saas": [ - # Duplicated entries - {"bk_app_code": bk_app.code, "module_name": "default"}, - {"bk_app_code": bk_app.code, "module_name": "default"}, - ] - } - response = api_client.post(url, request_body) + def test_upsert_duplicated_entries(self, api_client, bk_app, test_url): + response = api_client.post( + test_url, + { + "bk_saas": [ + # Duplicated entries + {"bk_app_code": bk_app.code, "module_name": "default"}, + {"bk_app_code": bk_app.code, "module_name": "default"}, + ] + }, + ) + assert response.status_code == 400 class TestDomainResolutionViewSet: - @pytest.fixture() - def domain_resolution(self, bk_app): - """创建一个 DomainResolution 对象""" - return DomainResolution.objects.create( - application=bk_app, - nameservers=["192.168.1.1", "192.168.1.2"], - host_aliases=[ + @pytest.fixture + def test_url(self, bk_app) -> str: + return reverse("api.applications.domain_resolution", kwargs={"code": bk_app.code}) + + @pytest.fixture + def with_default_res(self, api_client, test_url): + """创建一个默认的 DomainResolution 对象""" + body = { + "nameservers": ["192.168.1.1", "192.168.1.2"], + "host_aliases": [ { - "ip": "bk_app_code_test", + "ip": "127.0.0.1", "hostnames": [ - "bk_app_code_test", - "bk_app_code_test_x", + "foo.example.com", + "foo2.example.com", ], } ], - ) + } + resp = api_client.post(test_url, body) + return resp + + def test_get_missing(self, api_client, test_url): + response = api_client.get(test_url) + + assert response.status_code == 404 + + def test_get(self, with_default_res, api_client, bk_app, test_url): + response = api_client.get(test_url) - def test_get(self, api_client, bk_app, domain_resolution): - url = f"/api/bkapps/applications/{bk_app.code}/domain_resolution/" - response = api_client.get(url) assert response.status_code == 200 assert response.data["nameservers"] == ["192.168.1.1", "192.168.1.2"] assert response.data["host_aliases"] == [ { - "ip": "bk_app_code_test", + "ip": "127.0.0.1", "hostnames": [ - "bk_app_code_test", - "bk_app_code_test_x", + "foo.example.com", + "foo2.example.com", ], } ] - def test_get_error(self, api_client, bk_app): - url = f"/api/bkapps/applications/{bk_app.code}/domain_resolution/" - response = api_client.get(url) - assert response.status_code == 404 - @pytest.mark.parametrize( - ("request_body", "nameservers", "host_aliases"), + "req_body", [ - ( - { - "nameservers": ["192.168.1.3", "192.168.1.4"], - "host_aliases": [ - { - "ip": "1.1.1.1", - "hostnames": [ - "bk_app_code_test", - "bk_app_code_test_z", - ], - } - ], - }, - ["192.168.1.3", "192.168.1.4"], - [ - { - "ip": "1.1.1.1", - "hostnames": [ - "bk_app_code_test", - "bk_app_code_test_z", - ], - } - ], - ), - ( - { - "nameservers": ["192.168.1.3", "192.168.1.4"], - }, - ["192.168.1.3", "192.168.1.4"], - [], - ), - ( - { - "host_aliases": [ - { - "ip": "1.1.1.1", - "hostnames": [ - "bk_app_code_test", - "bk_app_code_test_z", - ], - } - ], - }, - [], - [ - { - "ip": "1.1.1.1", - "hostnames": [ - "bk_app_code_test", - "bk_app_code_test_z", - ], - } - ], - ), - ( - { - "nameservers": [], - "host_aliases": [], - }, - [], - [], - ), + { + "nameservers": ["192.168.1.100"], + "host_aliases": [{"ip": "8.8.8.8", "hostnames": ["bar.example.com"]}], + }, + # Only provide "nameserver" + { + "nameservers": ["192.168.1.100"], + }, + # Only provide "host_aliases" + { + "host_aliases": [{"ip": "8.8.8.8", "hostnames": ["bar.example.com"]}], + }, + # All fields are empty + { + "nameservers": [], + "host_aliases": [], + }, ], ) - def test_upsert(self, api_client, bk_app, domain_resolution, request_body, nameservers, host_aliases): - url = f"/api/bkapps/applications/{bk_app.code}/domain_resolution/" + def test_upsert(self, with_default_res, api_client, test_url, req_body): + old_ns = with_default_res.data["nameservers"] + old_ha = with_default_res.data["host_aliases"] + + response = api_client.post(test_url, req_body) - response = api_client.post(url, request_body) assert response.status_code == 200 - assert response.data["nameservers"] == nameservers or domain_resolution.nameservers - assert response.data["host_aliases"] == host_aliases or domain_resolution.nameservers + # When the field is missing in the body, the old value should be kept + assert response.data["nameservers"] == req_body.get("nameservers") or old_ns + assert response.data["host_aliases"] == req_body.get("host_aliases") or old_ha - def test_upsert_error(self, api_client, bk_app, domain_resolution): - url = f"/api/bkapps/applications/{bk_app.code}/domain_resolution/" + def test_upsert_no_data(self, with_default_res, api_client, test_url): + response = api_client.post(test_url) - response = api_client.post(url) assert response.status_code == 400 diff --git a/pyproject.toml b/pyproject.toml index 8a27c61a6d..ba8a741384 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -8,8 +8,7 @@ authors = ["blueking "] python = ">=3.11,<3.12" [tool.poetry.group.dev.dependencies] -ruff = "^0.6.9" -black = "24.3.0" +ruff = "^0.7.0" [tool.ruff] line-length = 119 From 12747a14d610e7428e5134d5dfb7e898c0ecb839 Mon Sep 17 00:00:00 2001 From: piglei Date: Tue, 22 Oct 2024 18:15:27 +0800 Subject: [PATCH 2/4] refactor: ignore PT001 --- .../paasng/tests/api/bkapp_model/test_network_config.py | 8 ++++---- pyproject.toml | 3 +++ 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/apiserver/paasng/tests/api/bkapp_model/test_network_config.py b/apiserver/paasng/tests/api/bkapp_model/test_network_config.py index ee42b6ba0c..6b8887f5b8 100644 --- a/apiserver/paasng/tests/api/bkapp_model/test_network_config.py +++ b/apiserver/paasng/tests/api/bkapp_model/test_network_config.py @@ -31,11 +31,11 @@ def bk_app2() -> Application: class TestSvcDiscConfigViewSet: - @pytest.fixture + @pytest.fixture() def test_url(self, bk_app) -> str: return reverse("api.applications.svc_disc", kwargs={"code": bk_app.code}) - @pytest.fixture + @pytest.fixture() def with_default_disc(self, api_client, test_url, bk_app2): """Create a default svc_disc object""" resp = api_client.post(test_url, {"bk_saas": [{"bk_app_code": bk_app2.code, "module_name": "default"}]}) @@ -87,11 +87,11 @@ def test_upsert_duplicated_entries(self, api_client, bk_app, test_url): class TestDomainResolutionViewSet: - @pytest.fixture + @pytest.fixture() def test_url(self, bk_app) -> str: return reverse("api.applications.domain_resolution", kwargs={"code": bk_app.code}) - @pytest.fixture + @pytest.fixture() def with_default_res(self, api_client, test_url): """创建一个默认的 DomainResolution 对象""" body = { diff --git a/pyproject.toml b/pyproject.toml index ba8a741384..03bc4199ca 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -83,6 +83,9 @@ ignore = [ "SIM108", # flake8-pytest-style：忽略后，允许 fixture 显式指定 scope="function" "PT003", + # flake8-pytest-style：忽略后，不再关注 @pytest.fixture 无参数时是否需要加括号 + "PT001", + # === 谨慎忽略 === # pep8-naming: 忽略后，不强制 class 必须使用驼峰命名，部分单元测试代码中用下划线起名可能更方便 From f067499f0f41d9c941a60c77c9266fac99c6c7a1 Mon Sep 17 00:00:00 2001 From: piglei Date: Tue, 22 Oct 2024 19:34:00 +0800 Subject: [PATCH 3/4] doc: add description for api tests --- apiserver/README.md | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/apiserver/README.md b/apiserver/README.md index 2ebfe50611..704cc5b79b 100644 --- a/apiserver/README.md +++ b/apiserver/README.md @@ -97,7 +97,7 @@ apiserver 项目的管理端（Admin42）使用 Nodejs 进行开发, 如需开 ## 测试 -项目的自动化测试基于 [pytest](https://docs.pytest.org/en/stable/) 框架编写，所有测试用例，可被笼统分为单元测试和 E2E 测试两类。 +项目的自动化测试基于 [pytest](https://docs.pytest.org/en/stable/) 框架编写，所有测试用例，可被笼统分为单元测试、API 测试和 E2E 测试三类。 #### 单元测试 @@ -121,6 +121,23 @@ $ pytest --reuse-db -s --maxfail=1 ./tests/ > 提示：每次提交代码改动前，请务必保证通过所有单元测试。 +#### API 测试 + +API 测试，指通过请求接口并验证响应是否符合预期的自动化测试。同单元测试相比，API 测试的速度通常更慢、依赖项更多，但是能覆盖更多的业务逻辑。 + +本项目的 API 测试代码主要位于 [./paasng/tests/api/](./paasng/tests/api/) 目录下。同传统的“黑盒 API 测试”（发送真实 HTTP 请求）有所不同，本项目的 API 测试是基于 Django/DRF 框架的 API 测试套件编写，并不发出真实网络请求，不过，这并不影响最终的测试效果。 + +一个典型的 API 测试，由“数据准备”、“发送请求”、“验证响应”这三个步骤组成。为了提升测试效果，让代码尽可能地便于维护，编码时请遵循以下建议： + +- 用例代码尽可能地简单，避免复杂逻辑； +- 尽量只通过调用 API 来完成测试； +- 减少依赖项，尽量避免 Mock，不直接操作数据模型； +- 除 bk_app 等 fixture 之外，不轻易引入其他模块代码； +- 避免直接调用各模块的功能函数（可以通过调 API 替代）； +- 使用 `reverse()` 函数获取请求路径，而不是硬编码字符串。 + +示例代码可参考：[./paasng/tests/api/bkapp_model/test_network_config.py](./paasng/tests/api/bkapp_model/test_network_config.py)。 + #### E2E 测试 E2E 测试是“端对端（End-to-end）测试”的缩写，特指那些需要访问真实的依赖服务才能正常运行的测试。E2E 测试运行速度慢，成本相比单元测试要高许多，比方说，运行测试前，你需要准备一个真实可用的 Kubernetes 集群（通常用 [kind](https://github.com/kubernetes-sigs/kind) 启动）。 From ee20f8ef6c00913e7523327962ae910cad24cb0a Mon Sep 17 00:00:00 2001 From: piglei Date: Fri, 25 Oct 2024 15:20:58 +0800 Subject: [PATCH 4/4] refactor: improve code in test_network_config --- apiserver/README.md | 2 +- .../tests/api/bkapp_model/test_network_config.py | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apiserver/README.md b/apiserver/README.md index 704cc5b79b..9e835e07eb 100644 --- a/apiserver/README.md +++ b/apiserver/README.md @@ -125,7 +125,7 @@ $ pytest --reuse-db -s --maxfail=1 ./tests/ API 测试，指通过请求接口并验证响应是否符合预期的自动化测试。同单元测试相比，API 测试的速度通常更慢、依赖项更多，但是能覆盖更多的业务逻辑。 -本项目的 API 测试代码主要位于 [./paasng/tests/api/](./paasng/tests/api/) 目录下。同传统的“黑盒 API 测试”（发送真实 HTTP 请求）有所不同，本项目的 API 测试是基于 Django/DRF 框架的 API 测试套件编写，并不发出真实网络请求，不过，这并不影响最终的测试效果。 +本项目的 API 测试代码主要位于 [./paasng/tests/api/](./paasng/tests/api/) 目录下。与传统的“黑盒 API 测试”（发送真实 HTTP 请求）有所不同，本项目的 API 测试是基于 Django/DRF 框架的 API 测试套件编写，并不发出真实网络请求，不过，这并不影响最终的测试效果。 一个典型的 API 测试，由“数据准备”、“发送请求”、“验证响应”这三个步骤组成。为了提升测试效果，让代码尽可能地便于维护，编码时请遵循以下建议： diff --git a/apiserver/paasng/tests/api/bkapp_model/test_network_config.py b/apiserver/paasng/tests/api/bkapp_model/test_network_config.py index 6b8887f5b8..9e1dbdbac6 100644 --- a/apiserver/paasng/tests/api/bkapp_model/test_network_config.py +++ b/apiserver/paasng/tests/api/bkapp_model/test_network_config.py @@ -152,15 +152,15 @@ def test_get(self, with_default_res, api_client, bk_app, test_url): ], ) def test_upsert(self, with_default_res, api_client, test_url, req_body): - old_ns = with_default_res.data["nameservers"] - old_ha = with_default_res.data["host_aliases"] - response = api_client.post(test_url, req_body) assert response.status_code == 200 - # When the field is missing in the body, the old value should be kept - assert response.data["nameservers"] == req_body.get("nameservers") or old_ns - assert response.data["host_aliases"] == req_body.get("host_aliases") or old_ha + + # The value of a field should has been updated when it's provided in the request + # body, otherwise it should be the same as before. + expected = with_default_res.data.copy() + expected.update(req_body) + assert response.data == expected def test_upsert_no_data(self, with_default_res, api_client, test_url): response = api_client.post(test_url)