From 385c8cb1d4992326f904782f84d3538da9b4280f Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Thu, 11 Jun 2026 10:15:44 -0400 Subject: [PATCH 01/12] remove submissionInstructionsMessage and submissionReceiptMessage from required attributes for Evaluation creation PUT request body. add test coverage. update documentation. --- synapseclient/models/evaluation.py | 24 ++++----- .../models/async/test_evaluation_async.py | 47 ++++++++++++++++ .../models/async/test_submission_async.py | 34 ++++++++++++ .../models/unit_test_evaluation.py | 54 ++++++++++--------- 4 files changed, 123 insertions(+), 36 deletions(-) diff --git a/synapseclient/models/evaluation.py b/synapseclient/models/evaluation.py index 5b34d32c3..6814d2ed0 100644 --- a/synapseclient/models/evaluation.py +++ b/synapseclient/models/evaluation.py @@ -33,11 +33,11 @@ class Evaluation(EvaluationSynchronousProtocol): etag: Synapse employs an Optimistic Concurrency Control (OCC) scheme to handle concurrent updates. The eTag changes every time an Evaluation is updated; it is used to detect when a client's copy of an Evaluation is out-of-date. - name: The name of this Evaluation. - description: A text description of this Evaluation. + name: **Required to store.** The name of this Evaluation. + description: **Required to store.** A text description of this Evaluation. owner_id: The ID of the Synapse user who created this Evaluation. created_on: The date on which Evaluation was created. - content_source: The Synapse ID of the Entity to which this Evaluation belongs, + content_source: **Required to store.** The Synapse ID of the Entity to which this Evaluation belongs, e.g. a reference to a Synapse project. submission_instructions_message: Message to display to users detailing acceptable formatting for Submissions to this Evaluation. submission_receipt_message: Message to display to users upon successful submission to this Evaluation. @@ -56,8 +56,6 @@ class Evaluation(EvaluationSynchronousProtocol): name="My Challenge Evaluation", description="Evaluation for my data challenge", content_source="syn123456", - submission_instructions_message="Submit CSV files only", - submission_receipt_message="Thank you for your submission!", ) created = evaluation.store() ``` @@ -123,10 +121,10 @@ class Evaluation(EvaluationSynchronousProtocol): of an Evaluation is out-of-date.""" name: Optional[str] = None - """The name of this Evaluation.""" + """**Required to store.** The name of this Evaluation.""" description: Optional[str] = None - """A text description of this Evaluation.""" + """**Required to store.** A text description of this Evaluation.""" owner_id: Optional[str] = None """The ID of the Synapse user who created this Evaluation.""" @@ -135,7 +133,7 @@ class Evaluation(EvaluationSynchronousProtocol): """The date on which Evaluation was created.""" content_source: Optional[str] = None - """The Synapse ID of the Entity to which this Evaluation belongs, + """**Required to store.** The Synapse ID of the Entity to which this Evaluation belongs, e.g. a reference to a Synapse project.""" submission_instructions_message: Optional[str] = None @@ -255,8 +253,6 @@ def to_synapse_request(self, request_type: RequestType): "name", "description", "content_source", - "submission_instructions_message", - "submission_receipt_message", ] # For "update" requests, add id and etag @@ -274,9 +270,13 @@ def to_synapse_request(self, request_type: RequestType): "name": self.name, "description": self.description, "contentSource": self.content_source, - "submissionInstructionsMessage": self.submission_instructions_message, - "submissionReceiptMessage": self.submission_receipt_message, } + if self.submission_instructions_message is not None: + request_body["submissionInstructionsMessage"] = ( + self.submission_instructions_message + ) + if self.submission_receipt_message is not None: + request_body["submissionReceiptMessage"] = self.submission_receipt_message # For UPDATE request types, add id and etag if request_type == RequestType.UPDATE: diff --git a/tests/integration/synapseclient/models/async/test_evaluation_async.py b/tests/integration/synapseclient/models/async/test_evaluation_async.py index d2a26e078..4deb8ef0e 100644 --- a/tests/integration/synapseclient/models/async/test_evaluation_async.py +++ b/tests/integration/synapseclient/models/async/test_evaluation_async.py @@ -46,6 +46,28 @@ async def test_create_evaluation(self): assert created_evaluation.owner_id is not None # Check that owner_id is set assert created_evaluation.created_on is not None # Check that created_on is set + async def test_create_evaluation_without_optional_message_fields(self): + # GIVEN a project to work with + project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( + synapse_client=self.syn + ) + self.schedule_for_cleanup(project.id) + + # WHEN I create an evaluation without submission_instructions_message or submission_receipt_message + evaluation = Evaluation( + name=f"test_evaluation_{uuid.uuid4()}", + description="A test evaluation without optional message fields", + content_source=project.id, + ) + created_evaluation = await evaluation.store_async(synapse_client=self.syn) + self.schedule_for_cleanup(created_evaluation.id) + + # THEN the evaluation should be created successfully + assert created_evaluation.id is not None + assert created_evaluation.etag is not None + assert created_evaluation.name == evaluation.name + assert created_evaluation.content_source == project.id + class TestGetEvaluation: @pytest.fixture(autouse=True, scope="function") @@ -294,6 +316,31 @@ async def test_update_multiple_fields(self, test_evaluation: Evaluation): assert updated_evaluation.etag is not None assert updated_evaluation.etag != old_etag + async def test_update_evaluation_without_optional_message_fields( + self, test_project: Project + ): + # GIVEN an evaluation created without submission message fields + evaluation = Evaluation( + name=f"test_evaluation_{uuid.uuid4()}", + description="A test evaluation without optional messages", + content_source=test_project.id, + ) + created_evaluation = await evaluation.store_async(synapse_client=self.syn) + self.schedule_for_cleanup(created_evaluation.id) + + # WHEN I update a field on that evaluation (still without message fields) + new_description = f"Updated description {uuid.uuid4()}" + created_evaluation.description = new_description + updated_evaluation = await created_evaluation.store_async( + synapse_client=self.syn + ) + + # THEN the update should succeed + assert updated_evaluation.description == new_description + assert updated_evaluation.id == created_evaluation.id + assert updated_evaluation.submission_instructions_message is None + assert updated_evaluation.submission_receipt_message is None + async def test_store_unchanged_evaluation( self, test_evaluation: Evaluation, monkeypatch ): diff --git a/tests/integration/synapseclient/models/async/test_submission_async.py b/tests/integration/synapseclient/models/async/test_submission_async.py index f8ae95f9c..71ca65f96 100644 --- a/tests/integration/synapseclient/models/async/test_submission_async.py +++ b/tests/integration/synapseclient/models/async/test_submission_async.py @@ -98,6 +98,40 @@ async def test_store_submission_successfully_async( assert created_submission.created_on is not None assert created_submission.version_number is not None + async def test_store_submission_to_evaluation_without_message_fields_async( + self, + test_project: Project, + test_file: File, + ): + # GIVEN an evaluation created without submission_instructions_message or submission_receipt_message + evaluation = Evaluation( + name=f"test_evaluation_{uuid.uuid4()}", + description="Evaluation without optional message fields", + content_source=test_project.id, + ) + created_evaluation = await evaluation.store_async(synapse_client=self.syn) + self.schedule_for_cleanup(created_evaluation.id) + + # WHEN I submit to that evaluation + submission = Submission( + entity_id=test_file.id, + evaluation_id=created_evaluation.id, + name=f"Test Submission {uuid.uuid4()}", + ) + created_submission = await submission.store_async(synapse_client=self.syn) + self.schedule_for_cleanup(created_submission.id) + + # THEN the submission should be created successfully + assert created_submission.id is not None + assert created_submission.entity_id == test_file.id + assert created_submission.evaluation_id == created_evaluation.id + + # AND retrieving the submission should also work + retrieved = await Submission(id=created_submission.id).get_async( + synapse_client=self.syn + ) + assert retrieved.id == created_submission.id + async def test_store_submission_without_entity_id_async( self, test_evaluation: Evaluation ): diff --git a/tests/unit/synapseclient/models/unit_test_evaluation.py b/tests/unit/synapseclient/models/unit_test_evaluation.py index 730e8771f..a1ae729eb 100644 --- a/tests/unit/synapseclient/models/unit_test_evaluation.py +++ b/tests/unit/synapseclient/models/unit_test_evaluation.py @@ -99,8 +99,6 @@ def test_to_synapse_request_missing_required_fields(self): Evaluation( description="Test", content_source="syn123", - submission_instructions_message="Inst", - submission_receipt_message="Rec", ), "name", ), @@ -108,8 +106,6 @@ def test_to_synapse_request_missing_required_fields(self): Evaluation( name="Test", content_source="syn123", - submission_instructions_message="Inst", - submission_receipt_message="Rec", ), "description", ), @@ -117,29 +113,9 @@ def test_to_synapse_request_missing_required_fields(self): Evaluation( name="Test", description="Test", - submission_instructions_message="Inst", - submission_receipt_message="Rec", ), "content_source", ), - ( - Evaluation( - name="Test", - description="Test", - content_source="syn123", - submission_receipt_message="Rec", - ), - "submission_instructions_message", - ), - ( - Evaluation( - name="Test", - description="Test", - content_source="syn123", - submission_instructions_message="Inst", - ), - "submission_receipt_message", - ), ] # Test each case @@ -149,6 +125,36 @@ def test_to_synapse_request_missing_required_fields(self): ): evaluation.to_synapse_request(RequestType.CREATE) + def test_to_synapse_request_without_optional_message_fields(self): + """Test that submission_instructions_message and submission_receipt_message + are not required — the request body omits them when absent.""" + # GIVEN an evaluation without message fields + evaluation = Evaluation( + name="Test Evaluation", + description="This is a test evaluation", + content_source="syn123456", + ) + + # WHEN we generate a request body for create + request_body = evaluation.to_synapse_request(RequestType.CREATE) + + # THEN the body should not include the optional message keys + assert request_body == { + "name": "Test Evaluation", + "description": "This is a test evaluation", + "contentSource": "syn123456", + } + assert "submissionInstructionsMessage" not in request_body + assert "submissionReceiptMessage" not in request_body + + # AND the same applies to an update request + evaluation.id = "9614112" + evaluation.etag = "abc-123-xyz" + update_body = evaluation.to_synapse_request(RequestType.UPDATE) + + assert "submissionInstructionsMessage" not in update_body + assert "submissionReceiptMessage" not in update_body + def test_set_last_persistent_instance(self): """Test setting the last persistent instance of an evaluation.""" # GIVEN a new evaluation From 5800f93a12158cb0e8551c0861d94066ddbfaf62 Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Fri, 12 Jun 2026 10:15:50 -0400 Subject: [PATCH 02/12] Address review comment: add class-level test_project fixture to TestEvaluationCreation Co-Authored-By: Claude Sonnet 4.6 --- .../models/async/test_evaluation_async.py | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/tests/integration/synapseclient/models/async/test_evaluation_async.py b/tests/integration/synapseclient/models/async/test_evaluation_async.py index 4deb8ef0e..2ea3cc9ad 100644 --- a/tests/integration/synapseclient/models/async/test_evaluation_async.py +++ b/tests/integration/synapseclient/models/async/test_evaluation_async.py @@ -17,18 +17,23 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - async def test_create_evaluation(self): - # GIVEN a project to work with + @pytest.fixture(scope="class") + async def test_project( + self, syn: Synapse, schedule_for_cleanup: Callable[..., None] + ) -> Project: + """Create a test project for evaluation creation tests.""" project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=self.syn + synapse_client=syn ) - self.schedule_for_cleanup(project.id) + schedule_for_cleanup(project.id) + return project + async def test_create_evaluation(self, test_project: Project): # WHEN I create an evaluation using the dataclass method evaluation = Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="A test evaluation for testing purposes", - content_source=project.id, + content_source=test_project.id, submission_instructions_message="Please submit your results in CSV format", submission_receipt_message="Thank you for your submission!", ) @@ -42,22 +47,18 @@ async def test_create_evaluation(self): assert ( created_evaluation.description == "A test evaluation for testing purposes" ) - assert created_evaluation.content_source == project.id + assert created_evaluation.content_source == test_project.id assert created_evaluation.owner_id is not None # Check that owner_id is set assert created_evaluation.created_on is not None # Check that created_on is set - async def test_create_evaluation_without_optional_message_fields(self): - # GIVEN a project to work with - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=self.syn - ) - self.schedule_for_cleanup(project.id) - + async def test_create_evaluation_without_optional_message_fields( + self, test_project: Project + ): # WHEN I create an evaluation without submission_instructions_message or submission_receipt_message evaluation = Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="A test evaluation without optional message fields", - content_source=project.id, + content_source=test_project.id, ) created_evaluation = await evaluation.store_async(synapse_client=self.syn) self.schedule_for_cleanup(created_evaluation.id) @@ -66,7 +67,7 @@ async def test_create_evaluation_without_optional_message_fields(self): assert created_evaluation.id is not None assert created_evaluation.etag is not None assert created_evaluation.name == evaluation.name - assert created_evaluation.content_source == project.id + assert created_evaluation.content_source == test_project.id class TestGetEvaluation: From ccb270949f9a91c938653eef4a68229783b7dfe1 Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Fri, 12 Jun 2026 10:28:15 -0400 Subject: [PATCH 03/12] Remove optional message fields from store_async docstring example Co-Authored-By: Claude Sonnet 4.6 --- synapseclient/models/evaluation.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapseclient/models/evaluation.py b/synapseclient/models/evaluation.py index 6814d2ed0..eb2ee8b00 100644 --- a/synapseclient/models/evaluation.py +++ b/synapseclient/models/evaluation.py @@ -307,7 +307,7 @@ async def store_async( Example: Creating a new evaluation   - Create a new evaluation on Synapse by storing an evaluation object with the required fields. If there are any fields missing, an error will be raised. + Create a new evaluation on Synapse by storing an evaluation object with the required fields. ```python from synapseclient.models import Evaluation from synapseclient import Synapse @@ -321,8 +321,6 @@ async def create_evaluation(): name="My Challenge Evaluation", description="Evaluation for my data challenge", content_source="syn123456", - submission_instructions_message="Submit CSV files only", - submission_receipt_message="Thank you for your submission!" ).store_async() return evaluation From 4531daff3f29684bb3e25683a457cd69ef841377 Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Wed, 24 Jun 2026 11:32:59 -0400 Subject: [PATCH 04/12] use delete_none_keys instead of conditional logic --- synapseclient/models/evaluation.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/synapseclient/models/evaluation.py b/synapseclient/models/evaluation.py index eb2ee8b00..da4f8f4c3 100644 --- a/synapseclient/models/evaluation.py +++ b/synapseclient/models/evaluation.py @@ -6,7 +6,7 @@ from synapseclient import Synapse from synapseclient.core.async_utils import async_to_sync -from synapseclient.core.utils import merge_dataclass_entities +from synapseclient.core.utils import delete_none_keys, merge_dataclass_entities from synapseclient.models.protocols.evaluation_protocol import ( EvaluationSynchronousProtocol, ) @@ -270,13 +270,10 @@ def to_synapse_request(self, request_type: RequestType): "name": self.name, "description": self.description, "contentSource": self.content_source, + "submissionInstructionsMessage": self.submission_instructions_message, + "submissionReceiptMessage": self.submission_receipt_message, } - if self.submission_instructions_message is not None: - request_body["submissionInstructionsMessage"] = ( - self.submission_instructions_message - ) - if self.submission_receipt_message is not None: - request_body["submissionReceiptMessage"] = self.submission_receipt_message + delete_none_keys(request_body) # For UPDATE request types, add id and etag if request_type == RequestType.UPDATE: From a781a94a3e4b23b72c04430c8837dc00f4b68a08 Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Wed, 24 Jun 2026 11:33:58 -0400 Subject: [PATCH 05/12] merge test_create_eval... and test_update_eval... and add check for None submission msgs --- .../models/async/test_evaluation_async.py | 40 +++++++------------ 1 file changed, 15 insertions(+), 25 deletions(-) diff --git a/tests/integration/synapseclient/models/async/test_evaluation_async.py b/tests/integration/synapseclient/models/async/test_evaluation_async.py index 2ea3cc9ad..7835b0b2c 100644 --- a/tests/integration/synapseclient/models/async/test_evaluation_async.py +++ b/tests/integration/synapseclient/models/async/test_evaluation_async.py @@ -68,6 +68,21 @@ async def test_create_evaluation_without_optional_message_fields( assert created_evaluation.etag is not None assert created_evaluation.name == evaluation.name assert created_evaluation.content_source == test_project.id + assert created_evaluation.submission_instructions_message is None + assert created_evaluation.submission_receipt_message is None + + # WHEN I update a field without setting message fields + new_description = f"Updated description {uuid.uuid4()}" + old_etag = created_evaluation.etag + created_evaluation.description = new_description + updated_evaluation = await created_evaluation.store_async(synapse_client=self.syn) + + # THEN the update should also succeed without message fields + assert updated_evaluation.description == new_description + assert updated_evaluation.id == created_evaluation.id + assert updated_evaluation.etag != old_etag + assert updated_evaluation.submission_instructions_message is None + assert updated_evaluation.submission_receipt_message is None class TestGetEvaluation: @@ -317,31 +332,6 @@ async def test_update_multiple_fields(self, test_evaluation: Evaluation): assert updated_evaluation.etag is not None assert updated_evaluation.etag != old_etag - async def test_update_evaluation_without_optional_message_fields( - self, test_project: Project - ): - # GIVEN an evaluation created without submission message fields - evaluation = Evaluation( - name=f"test_evaluation_{uuid.uuid4()}", - description="A test evaluation without optional messages", - content_source=test_project.id, - ) - created_evaluation = await evaluation.store_async(synapse_client=self.syn) - self.schedule_for_cleanup(created_evaluation.id) - - # WHEN I update a field on that evaluation (still without message fields) - new_description = f"Updated description {uuid.uuid4()}" - created_evaluation.description = new_description - updated_evaluation = await created_evaluation.store_async( - synapse_client=self.syn - ) - - # THEN the update should succeed - assert updated_evaluation.description == new_description - assert updated_evaluation.id == created_evaluation.id - assert updated_evaluation.submission_instructions_message is None - assert updated_evaluation.submission_receipt_message is None - async def test_store_unchanged_evaluation( self, test_evaluation: Evaluation, monkeypatch ): From d1b36efbedf96e5ea8351d74827f78e78d58711e Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Wed, 24 Jun 2026 11:35:05 -0400 Subject: [PATCH 06/12] formatting --- .../synapseclient/models/async/test_evaluation_async.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/synapseclient/models/async/test_evaluation_async.py b/tests/integration/synapseclient/models/async/test_evaluation_async.py index 7835b0b2c..13ef7304b 100644 --- a/tests/integration/synapseclient/models/async/test_evaluation_async.py +++ b/tests/integration/synapseclient/models/async/test_evaluation_async.py @@ -75,7 +75,9 @@ async def test_create_evaluation_without_optional_message_fields( new_description = f"Updated description {uuid.uuid4()}" old_etag = created_evaluation.etag created_evaluation.description = new_description - updated_evaluation = await created_evaluation.store_async(synapse_client=self.syn) + updated_evaluation = await created_evaluation.store_async( + synapse_client=self.syn + ) # THEN the update should also succeed without message fields assert updated_evaluation.description == new_description From 149e61fd9b3a927c19fe4aabdbe7f88e184d84be Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Wed, 24 Jun 2026 11:43:21 -0400 Subject: [PATCH 07/12] use session-scoped project shared across all tests --- .../models/async/test_evaluation_async.py | 23 +++++-------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/tests/integration/synapseclient/models/async/test_evaluation_async.py b/tests/integration/synapseclient/models/async/test_evaluation_async.py index 13ef7304b..336b90b1b 100644 --- a/tests/integration/synapseclient/models/async/test_evaluation_async.py +++ b/tests/integration/synapseclient/models/async/test_evaluation_async.py @@ -17,23 +17,12 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - @pytest.fixture(scope="class") - async def test_project( - self, syn: Synapse, schedule_for_cleanup: Callable[..., None] - ) -> Project: - """Create a test project for evaluation creation tests.""" - project = await Project(name=f"test_project_{uuid.uuid4()}").store_async( - synapse_client=syn - ) - schedule_for_cleanup(project.id) - return project - - async def test_create_evaluation(self, test_project: Project): + async def test_create_evaluation(self, project_model: Project): # WHEN I create an evaluation using the dataclass method evaluation = Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="A test evaluation for testing purposes", - content_source=test_project.id, + content_source=project_model.id, submission_instructions_message="Please submit your results in CSV format", submission_receipt_message="Thank you for your submission!", ) @@ -47,18 +36,18 @@ async def test_create_evaluation(self, test_project: Project): assert ( created_evaluation.description == "A test evaluation for testing purposes" ) - assert created_evaluation.content_source == test_project.id + assert created_evaluation.content_source == project_model.id assert created_evaluation.owner_id is not None # Check that owner_id is set assert created_evaluation.created_on is not None # Check that created_on is set async def test_create_evaluation_without_optional_message_fields( - self, test_project: Project + self, project_model: Project ): # WHEN I create an evaluation without submission_instructions_message or submission_receipt_message evaluation = Evaluation( name=f"test_evaluation_{uuid.uuid4()}", description="A test evaluation without optional message fields", - content_source=test_project.id, + content_source=project_model.id, ) created_evaluation = await evaluation.store_async(synapse_client=self.syn) self.schedule_for_cleanup(created_evaluation.id) @@ -67,7 +56,7 @@ async def test_create_evaluation_without_optional_message_fields( assert created_evaluation.id is not None assert created_evaluation.etag is not None assert created_evaluation.name == evaluation.name - assert created_evaluation.content_source == test_project.id + assert created_evaluation.content_source == project_model.id assert created_evaluation.submission_instructions_message is None assert created_evaluation.submission_receipt_message is None From e3c0b3c305ae562507aa4f9e6098737146eae159 Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Wed, 24 Jun 2026 15:31:09 -0400 Subject: [PATCH 08/12] build with all fields not required fields --- .../synapseclient/models/async/unit_test_evaluation_async.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/synapseclient/models/async/unit_test_evaluation_async.py b/tests/unit/synapseclient/models/async/unit_test_evaluation_async.py index 71ee2346f..def3f2e96 100644 --- a/tests/unit/synapseclient/models/async/unit_test_evaluation_async.py +++ b/tests/unit/synapseclient/models/async/unit_test_evaluation_async.py @@ -92,7 +92,7 @@ def get_example_permissions_response(self) -> Dict[str, Union[str, List, bool]]: } def _build_evaluation_with_all_fields(self) -> Evaluation: - """Build an Evaluation instance with all required fields for store.""" + """Build an Evaluation instance with all fields set.""" return Evaluation( name=EVALUATION_NAME, description=EVALUATION_DESCRIPTION, From 89dd01207f2b7c96d4c76504bae599955efcaa0f Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Wed, 24 Jun 2026 15:31:25 -0400 Subject: [PATCH 09/12] add other missing field scenarios --- .../models/async/test_evaluation_async.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/tests/integration/synapseclient/models/async/test_evaluation_async.py b/tests/integration/synapseclient/models/async/test_evaluation_async.py index 336b90b1b..51ddb263a 100644 --- a/tests/integration/synapseclient/models/async/test_evaluation_async.py +++ b/tests/integration/synapseclient/models/async/test_evaluation_async.py @@ -556,12 +556,20 @@ def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: self.syn = syn self.schedule_for_cleanup = schedule_for_cleanup - async def test_create_evaluation_missing_required_fields(self): - # WHEN I try to create an evaluation with missing required fields - evaluation = Evaluation(name="test_evaluation") - + @pytest.mark.parametrize( + "evaluation,missing_field", + [ + (Evaluation(description="Test", content_source="syn123"), "name"), + (Evaluation(name="Test", content_source="syn123"), "description"), + (Evaluation(name="Test", description="Test"), "content_source"), + ], + ) + async def test_create_evaluation_missing_required_fields( + self, evaluation: Evaluation, missing_field: str + ): + # WHEN I try to create an evaluation with a missing required field # THEN it should raise a ValueError - with pytest.raises(ValueError, match="missing the 'description' attribute"): + with pytest.raises(ValueError, match=f"missing the '{missing_field}' attribute"): await evaluation.store_async(synapse_client=self.syn) async def test_get_evaluation_missing_id_and_name(self): From b6ccf3b183a3261b7c3e150eb579b0653d77327b Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Wed, 24 Jun 2026 15:33:25 -0400 Subject: [PATCH 10/12] refactored to_synapse_request to use per-field checks --- synapseclient/models/evaluation.py | 31 +++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/synapseclient/models/evaluation.py b/synapseclient/models/evaluation.py index da4f8f4c3..105564ece 100644 --- a/synapseclient/models/evaluation.py +++ b/synapseclient/models/evaluation.py @@ -248,21 +248,26 @@ def to_synapse_request(self, request_type: RequestType): ValueError: If any required attributes are missing. """ - # These attributes are required in our PUT requests for creating or updating an evaluation - required_attributes = [ - "name", - "description", - "content_source", - ] - - # For "update" requests, add id and etag + if not self.name: + raise ValueError( + f"Your evaluation object is missing the 'name' attribute. This attribute is required to {request_type.value} an evaluation" + ) + if not self.description: + raise ValueError( + f"Your evaluation object is missing the 'description' attribute. This attribute is required to {request_type.value} an evaluation" + ) + if not self.content_source: + raise ValueError( + f"Your evaluation object is missing the 'content_source' attribute. This attribute is required to {request_type.value} an evaluation" + ) if request_type == RequestType.UPDATE: - required_attributes.extend(["id", "etag"]) - - for attribute in required_attributes: - if not getattr(self, attribute): + if not self.id: + raise ValueError( + f"Your evaluation object is missing the 'id' attribute. This attribute is required to {request_type.value} an evaluation" + ) + if not self.etag: raise ValueError( - f"Your evaluation object is missing the '{attribute}' attribute. This attribute is required to {request_type.value} an evaluation" + f"Your evaluation object is missing the 'etag' attribute. This attribute is required to {request_type.value} an evaluation" ) # Build a request body for storing a brand new evaluation From 7cc9628c0512e559392a00ae70fb46c41f95e22d Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Wed, 24 Jun 2026 15:38:56 -0400 Subject: [PATCH 11/12] refactor: move TestEvaluationValidation to unit test suite --- .../models/async/test_evaluation_async.py | 57 ------------------- .../models/unit_test_evaluation.py | 52 +++++++++++++++++ 2 files changed, 52 insertions(+), 57 deletions(-) diff --git a/tests/integration/synapseclient/models/async/test_evaluation_async.py b/tests/integration/synapseclient/models/async/test_evaluation_async.py index 51ddb263a..cc9ccb46c 100644 --- a/tests/integration/synapseclient/models/async/test_evaluation_async.py +++ b/tests/integration/synapseclient/models/async/test_evaluation_async.py @@ -550,60 +550,3 @@ async def test_update_acl_async_with_full_dictionary( assert set(user_access["accessType"]) == set(["READ", "DELETE", "SUBMIT"]) -class TestEvaluationValidation: - @pytest.fixture(autouse=True, scope="function") - def init(self, syn: Synapse, schedule_for_cleanup: Callable[..., None]) -> None: - self.syn = syn - self.schedule_for_cleanup = schedule_for_cleanup - - @pytest.mark.parametrize( - "evaluation,missing_field", - [ - (Evaluation(description="Test", content_source="syn123"), "name"), - (Evaluation(name="Test", content_source="syn123"), "description"), - (Evaluation(name="Test", description="Test"), "content_source"), - ], - ) - async def test_create_evaluation_missing_required_fields( - self, evaluation: Evaluation, missing_field: str - ): - # WHEN I try to create an evaluation with a missing required field - # THEN it should raise a ValueError - with pytest.raises(ValueError, match=f"missing the '{missing_field}' attribute"): - await evaluation.store_async(synapse_client=self.syn) - - async def test_get_evaluation_missing_id_and_name(self): - # WHEN I try to get an evaluation without id or name - evaluation = Evaluation() - - # THEN it should raise a ValueError - with pytest.raises( - ValueError, match="Either id or name must be set to get an evaluation" - ): - await evaluation.get_async(synapse_client=self.syn) - - async def test_delete_evaluation_missing_id(self): - # WHEN I try to delete an evaluation without an id - evaluation = Evaluation(name="test_evaluation") - - # THEN it should raise a ValueError - with pytest.raises(ValueError, match="id must be set to delete an evaluation"): - await evaluation.delete_async(synapse_client=self.syn) - - async def test_get_acl_missing_id(self): - # WHEN I try to get ACL for an evaluation without an id - evaluation = Evaluation(name="test_evaluation") - - # THEN it should raise a ValueError - with pytest.raises(ValueError, match="id must be set to get evaluation ACL"): - await evaluation.get_acl_async(synapse_client=self.syn) - - async def test_get_permissions_missing_id(self): - # WHEN I try to get permissions for an evaluation without an id - evaluation = Evaluation(name="test_evaluation") - - # THEN it should raise a ValueError - with pytest.raises( - ValueError, match="id must be set to get evaluation permissions" - ): - await evaluation.get_permissions_async(synapse_client=self.syn) diff --git a/tests/unit/synapseclient/models/unit_test_evaluation.py b/tests/unit/synapseclient/models/unit_test_evaluation.py index a1ae729eb..9f0aa70b3 100644 --- a/tests/unit/synapseclient/models/unit_test_evaluation.py +++ b/tests/unit/synapseclient/models/unit_test_evaluation.py @@ -583,3 +583,55 @@ def test_log_message_when_removing_principal(self): mock_logger.info.assert_called_once_with( "Principal ID 12345 will be removed from ACL due to empty access_type" ) + + @pytest.mark.parametrize( + "evaluation,missing_field", + [ + (Evaluation(description="Test", content_source="syn123"), "name"), + (Evaluation(name="Test", content_source="syn123"), "description"), + (Evaluation(name="Test", description="Test"), "content_source"), + ], + ) + async def test_create_evaluation_missing_required_fields( + self, evaluation: Evaluation, missing_field: str, syn: Synapse + ): + # WHEN I try to create an evaluation with a missing required field + # THEN it should raise a ValueError + with pytest.raises(ValueError, match=f"missing the '{missing_field}' attribute"): + await evaluation.store_async(synapse_client=syn) + + async def test_get_evaluation_missing_id_and_name(self, syn: Synapse): + # WHEN I try to get an evaluation without id or name + evaluation = Evaluation() + + # THEN it should raise a ValueError + with pytest.raises( + ValueError, match="Either id or name must be set to get an evaluation" + ): + await evaluation.get_async(synapse_client=syn) + + async def test_delete_evaluation_missing_id(self, syn: Synapse): + # WHEN I try to delete an evaluation without an id + evaluation = Evaluation(name="test_evaluation") + + # THEN it should raise a ValueError + with pytest.raises(ValueError, match="id must be set to delete an evaluation"): + await evaluation.delete_async(synapse_client=syn) + + async def test_get_acl_missing_id(self, syn: Synapse): + # WHEN I try to get ACL for an evaluation without an id + evaluation = Evaluation(name="test_evaluation") + + # THEN it should raise a ValueError + with pytest.raises(ValueError, match="id must be set to get evaluation ACL"): + await evaluation.get_acl_async(synapse_client=syn) + + async def test_get_permissions_missing_id(self, syn: Synapse): + # WHEN I try to get permissions for an evaluation without an id + evaluation = Evaluation(name="test_evaluation") + + # THEN it should raise a ValueError + with pytest.raises( + ValueError, match="id must be set to get evaluation permissions" + ): + await evaluation.get_permissions_async(synapse_client=syn) From e05dd846ef19c87639f19c46c9fe6aae6569f626 Mon Sep 17 00:00:00 2001 From: Jenny Medina Date: Wed, 24 Jun 2026 15:42:19 -0400 Subject: [PATCH 12/12] refactor: move TestEvaluationValidation to unit test suite --- .../synapseclient/models/async/test_evaluation_async.py | 2 -- tests/unit/synapseclient/models/unit_test_evaluation.py | 4 +++- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/synapseclient/models/async/test_evaluation_async.py b/tests/integration/synapseclient/models/async/test_evaluation_async.py index cc9ccb46c..4918ee4ff 100644 --- a/tests/integration/synapseclient/models/async/test_evaluation_async.py +++ b/tests/integration/synapseclient/models/async/test_evaluation_async.py @@ -548,5 +548,3 @@ async def test_update_acl_async_with_full_dictionary( user_access is not None ), f"User {current_user_id} not found in updated ACL" assert set(user_access["accessType"]) == set(["READ", "DELETE", "SUBMIT"]) - - diff --git a/tests/unit/synapseclient/models/unit_test_evaluation.py b/tests/unit/synapseclient/models/unit_test_evaluation.py index 9f0aa70b3..2851ef2ae 100644 --- a/tests/unit/synapseclient/models/unit_test_evaluation.py +++ b/tests/unit/synapseclient/models/unit_test_evaluation.py @@ -597,7 +597,9 @@ async def test_create_evaluation_missing_required_fields( ): # WHEN I try to create an evaluation with a missing required field # THEN it should raise a ValueError - with pytest.raises(ValueError, match=f"missing the '{missing_field}' attribute"): + with pytest.raises( + ValueError, match=f"missing the '{missing_field}' attribute" + ): await evaluation.store_async(synapse_client=syn) async def test_get_evaluation_missing_id_and_name(self, syn: Synapse):