refactor: improve test code in test_network_config.py and update docs (…

…#1673)

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) 启动）。

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",
-                }
-            ],
-        )
+    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=[
+    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):
+        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
 
-    def test_upsert_error(self, api_client, bk_app, domain_resolution):
-        url = f"/api/bkapps/applications/{bk_app.code}/domain_resolution/"
+        # 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)
 
-        response = api_client.post(url)
         assert response.status_code == 400

pyproject.toml

@@ -8,8 +8,7 @@ authors = ["blueking <blueking@tencent.com>"]
 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
@@ -84,6 +83,9 @@ ignore = [
   "SIM108",
   # flake8-pytest-style：忽略后，允许 fixture 显式指定 scope="function"
   "PT003",
+  # flake8-pytest-style：忽略后，不再关注 @pytest.fixture 无参数时是否需要加括号
+  "PT001",
+
 
   # === 谨慎忽略 ===
   # pep8-naming: 忽略后，不强制 class 必须使用驼峰命名，部分单元测试代码中用下划线起名可能更方便