From 95444cafe2fd81fcee89986154ce398c2b232be0 Mon Sep 17 00:00:00 2001 From: Iain Dillingham Date: Mon, 7 Oct 2024 09:58:19 +0100 Subject: [PATCH 1/3] Rename test This renames a test to better reflect its intent. --- tests/unit/jobserver/test_forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/jobserver/test_forms.py b/tests/unit/jobserver/test_forms.py index b0db85faf..9ac16bd52 100644 --- a/tests/unit/jobserver/test_forms.py +++ b/tests/unit/jobserver/test_forms.py @@ -116,7 +116,7 @@ def test_workspacecreateform_success(): assert form.is_valid() -def test_workspacecreateform_success_with_upper_case_names(): +def test_workspacecreateform_success_with_mixed_case_name(): project = ProjectFactory() data = { "name": "TeSt", From 651c9e6ff2bde8a1d94e713773b58203c75b473b Mon Sep 17 00:00:00 2001 From: Iain Dillingham Date: Mon, 7 Oct 2024 13:46:37 +0100 Subject: [PATCH 2/3] Assert `name` field is cleaned This adds an assertion to `test_workspacecreateform_success` that matches the assertion in `test_workspacecreateform_success_with_mixed_case_name`. In both cases, the assertions check that the `name` field is cleaned. It wasn't clear to me why the assertion message was the cleaned `name` field (possibly for debugging?), so I removed the assertion message. --- tests/unit/jobserver/test_forms.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/unit/jobserver/test_forms.py b/tests/unit/jobserver/test_forms.py index 9ac16bd52..757d4abd4 100644 --- a/tests/unit/jobserver/test_forms.py +++ b/tests/unit/jobserver/test_forms.py @@ -114,6 +114,7 @@ def test_workspacecreateform_success(): form = WorkspaceCreateForm(project, repos_with_branches, data) assert form.is_valid() + assert form.cleaned_data["name"] == "test" def test_workspacecreateform_success_with_mixed_case_name(): @@ -134,7 +135,7 @@ def test_workspacecreateform_success_with_mixed_case_name(): form = WorkspaceCreateForm(project, repos_with_branches, data) assert form.is_valid() - assert form.cleaned_data["name"] == "test", form.cleaned_data["name"] + assert form.cleaned_data["name"] == "test" def test_workspacecreateform_unknown_branch(): From ada412bd2f652bf92ff7880606a7083acdae1f3c Mon Sep 17 00:00:00 2001 From: Iain Dillingham Date: Mon, 7 Oct 2024 15:12:34 +0100 Subject: [PATCH 3/3] Parametrize test If you place them side-by-side, then you should be able to see that `test_workspacecreateform_success` and `test_workspacecreateform_success_with_mixed_case_name` are identical, save for the value of `name`. Let's parametrize the former and remove the latter, using `pytest.param` to highlight what's important about each parameter set. --- tests/unit/jobserver/test_forms.py | 34 +++++++++--------------------- 1 file changed, 10 insertions(+), 24 deletions(-) diff --git a/tests/unit/jobserver/test_forms.py b/tests/unit/jobserver/test_forms.py index 757d4abd4..c31e8a9c8 100644 --- a/tests/unit/jobserver/test_forms.py +++ b/tests/unit/jobserver/test_forms.py @@ -96,31 +96,17 @@ def test_jobrequestcreateform_with_bad_codelists( ) -def test_workspacecreateform_success(): - project = ProjectFactory() - data = { - "name": "test", - "repo": "http://example.com/derp/test-repo", - "branch": "test-branch", - "purpose": "test", - } - repos_with_branches = [ - { - "name": "test-repo", - "url": "http://example.com/derp/test-repo", - "branches": ["test-branch"], - } - ] - form = WorkspaceCreateForm(project, repos_with_branches, data) - - assert form.is_valid() - assert form.cleaned_data["name"] == "test" - - -def test_workspacecreateform_success_with_mixed_case_name(): +@pytest.mark.parametrize( + "name,cleaned_name", + [ + pytest.param("test", "test", id="lower_case_name"), + pytest.param("TeSt", "test", id="mixed_case_name"), + ], +) +def test_workspacecreateform_success(name, cleaned_name): project = ProjectFactory() data = { - "name": "TeSt", + "name": name, "repo": "http://example.com/derp/test-repo", "branch": "test-branch", "purpose": "test", @@ -135,7 +121,7 @@ def test_workspacecreateform_success_with_mixed_case_name(): form = WorkspaceCreateForm(project, repos_with_branches, data) assert form.is_valid() - assert form.cleaned_data["name"] == "test" + assert form.cleaned_data["name"] == cleaned_name def test_workspacecreateform_unknown_branch():