Standardize mock_response_factory usage in test_base.py and update documentation

- Updated test_base.py to use mock_response_factory pattern
- Enhanced docs/DEVELOPMENT.md with detailed examples for mock_response_factory usage
- Documented both standard response pattern and parameter validation pattern

Author: jpstroop

docs/DEVELOPMENT.md

@@ -195,27 +195,81 @@ All resource mocks are in the root [conftest.py](tests/conftest.py).
 
 ### Response Mocking
 
-The test suite uses the `mock_response_factory` fixture to create consistent,
-configurable mock responses:
+The test suite uses the `mock_response_factory` fixture from `tests/conftest.py`
+to create consistent, configurable mock responses. This is the required pattern
+for all tests that need to mock HTTP responses.
+
+#### Standard Mock Response Pattern
+
+```python
+def test_some_endpoint(resource, mock_oauth_session, mock_response_factory):
+    """Test description."""
+    # Define expected data first
+    expected_data = {"data": "test"}
+    
+    # Create mock response using the factory
+    mock_response = mock_response_factory(200, expected_data)
+    
+    # Assign to oauth.request.return_value
+    resource.oauth.request.return_value = mock_response
+    
+    # Call the method under test
+    result = resource.some_method()
+    
+    # Assert against expected_data, not mock_response.json.return_value
+    assert result == expected_data
+```
+
+#### Response Factory Examples
 
 ```python
-# Creating a mock response with status code and data
+# Success response with data
 mock_response = mock_response_factory(200, {"data": "test"})
 
-# Creating a mock response with additional headers
+# Response with custom headers
 mock_response = mock_response_factory(
     status_code=200,
     json_data={"data": "test"},
     headers={"custom-header": "value"}
 )
 
-# Creating a delete response with no content (204)
+# Delete/no content response (204)
 mock_response = mock_response_factory(204)
+
+# Error response
+mock_response = mock_response_factory(
+    400, 
+    {"errors": [{"errorType": "validation", "message": "Error message"}]}
+)
+
+# Non-JSON response (XML)
+mock_response = mock_response_factory(
+    200, 
+    headers={"content-type": "application/vnd.garmin.tcx+xml"},
+    content_type="application/vnd.garmin.tcx+xml"
+)
+mock_response.text = "<xml>content</xml>"
+```
+
+#### Parameter Validation Pattern
+
+For tests that only need to verify parameter validation or endpoint construction
+(not response handling), it's acceptable to use the following alternative
+pattern:
+
+```python
+def test_validation(resource):
+    """Test parameter validation."""
+    resource._make_request = Mock()
+    resource.some_method(param="value")
+    resource._make_request.assert_called_once_with(
+        "endpoint/path", params={"param": "value"}, user_id="-", debug=False
+    )
 ```
 
 This approach provides a clean, standardized way to create mock responses with
-the desired status code, data, and headers. All test files should use this
-factory method rather than manually configuring mock responses.
+the desired status code, data, and headers. All test files must use one of these
+patterns.
 
 ## OAuth Callback Implementation
 

standardize_mock_responses.py

@@ -114,14 +114,14 @@ def fix_mock_response_usage(match):
         return test_func
 
     content = re.sub(pattern5, fix_mock_response_usage, content, flags=re.DOTALL)
-    
+
     # Pattern 6: Replace assert result == mock_response.json.return_value with assert result == expected_data
-    pattern6 = r'([ \t]*assert\s+.*?==\s*)mock_response\.json\.return_value'
-    
+    pattern6 = r"([ \t]*assert\s+.*?==\s*)mock_response\.json\.return_value"
+
     def fix_assertions(match):
         before_part = match.group(1)
         return f"{before_part}mock_response.json()"
-    
+
     content = re.sub(pattern6, fix_assertions, content)
 
     # If no changes were made, return False
@@ -143,55 +143,57 @@ def find_files_to_transform() -> List[Tuple[str, bool]]:
     if len(sys.argv) > 1 and not sys.argv[1].startswith("--"):
         test_dir = Path(sys.argv[1])
         print(f"Searching in directory: {test_dir}")
-    
+
     result = []
     verbose = "--verbose" in sys.argv
-    
+
     for root, _, files in os.walk(test_dir):
         for file in files:
             if file.endswith(".py") and file.startswith("test_"):
                 file_path = os.path.join(root, file)
-                
+
                 # First check for any mention of mock_response
                 has_mock_response = bool(find_pattern_in_file(file_path, r"mock_response"))
-                
+
                 if not has_mock_response:
                     continue
-                
+
                 # More detailed patterns
                 # Check if file uses mock_response directly as parameter
                 uses_mock_response = bool(
                     find_pattern_in_file(
                         file_path, r"def\s+test_\w+\([^)]*,\s*mock_response\s*[,)]"
                     )
                 )
-                
+
                 # Check if file directly manipulates mock_response
                 manipulates_mock_response = bool(
                     find_pattern_in_file(
                         file_path, r"mock_response\.(json\.return_value|status_code)\s*="
                     )
                 )
-                
+
                 # Check if file uses mock_response.json.return_value in assertions
                 uses_return_value_in_assertions = bool(
                     find_pattern_in_file(
                         file_path, r"assert\s+.*?=.*?mock_response\.json\.return_value"
                     )
                 )
-                
+
                 # Check if file uses mock_response_factory
                 uses_factory = bool(
                     find_pattern_in_file(file_path, r"mock_response\s*=\s*mock_response_factory")
                 )
-                
+
                 # Determine if file needs transformation
-                needs_transform = (uses_mock_response or manipulates_mock_response) and not uses_factory
-                
+                needs_transform = (
+                    uses_mock_response or manipulates_mock_response
+                ) and not uses_factory
+
                 # Also flag files that use factory but still use mock_response.json.return_value in assertions
                 if uses_factory and uses_return_value_in_assertions and not needs_transform:
                     needs_transform = True
-                
+
                 if verbose or needs_transform:
                     print(f"File: {file_path}")
                     print(f"  Has mock_response: {has_mock_response}")
@@ -200,7 +202,7 @@ def find_files_to_transform() -> List[Tuple[str, bool]]:
                     print(f"  Uses in assertions: {uses_return_value_in_assertions}")
                     print(f"  Uses factory: {uses_factory}")
                     print(f"  Needs transform: {needs_transform}")
-                
+
                 if needs_transform:
                     result.append((file_path, needs_transform))
 

tests/fitbit_client/resources/test_base.py

@@ -258,17 +258,18 @@ def test_log_response_for_error_without_content(base_resource, mock_logger):
 # -----------------------------------------------------------------------------
 
 
-def test_handle_json_response(base_resource, mock_response):
+def test_handle_json_response(base_resource, mock_response_factory):
     """Test JSON response handling"""
-    mock_response.json.return_value = {"data": "test"}
-    mock_response.status_code = 200
+    expected_data = {"data": "test"}
+    mock_response = mock_response_factory(200, expected_data)
 
     result = base_resource._handle_json_response("test_method", "test/endpoint", mock_response)
-    assert result == {"data": "test"}
+    assert result == expected_data
 
 
-def test_handle_json_response_invalid(base_resource, mock_response):
+def test_handle_json_response_invalid(base_resource, mock_response_factory):
     """Test invalid JSON handling"""
+    mock_response = mock_response_factory(200)
     mock_response.json.side_effect = JSONDecodeError("Invalid JSON", "doc", 0)
     mock_response.text = "Invalid {json"
 
@@ -295,44 +296,49 @@ def test_handle_json_response_with_invalid_json(base_resource, mock_logger):
 # -----------------------------------------------------------------------------
 
 
-def test_make_request_json_success(base_resource, mock_oauth_session, mock_response):
+def test_make_request_json_success(base_resource, mock_oauth_session, mock_response_factory):
     """Test successful JSON request"""
-    mock_response.json.return_value = {"success": True}
-    mock_response.headers = {"content-type": "application/json"}
-    mock_response.status_code = 200
+    expected_data = {"success": True}
+    mock_response = mock_response_factory(
+        200, expected_data, headers={"content-type": "application/json"}
+    )
     mock_oauth_session.request.return_value = mock_response
 
     result = base_resource._make_request("test/endpoint")
-    assert result == {"success": True}
+    assert result == expected_data
 
 
-def test_make_request_no_content(base_resource, mock_oauth_session, mock_response):
+def test_make_request_no_content(base_resource, mock_oauth_session, mock_response_factory):
     """Test request with no content"""
-    mock_response.status_code = 204
-    mock_response.headers = {}
-    mock_response.json.return_value = {"success": True}
+    mock_response = mock_response_factory(204, headers={})
     mock_oauth_session.request.return_value = mock_response
 
     result = base_resource._make_request("test/endpoint")
     assert result is None
 
 
-def test_make_request_xml_response(base_resource, mock_oauth_session, mock_response):
+def test_make_request_xml_response(base_resource, mock_oauth_session, mock_response_factory):
     """Test XML response handling"""
+    mock_response = mock_response_factory(
+        200,
+        headers={"content-type": "application/vnd.garmin.tcx+xml"},
+        content_type="application/vnd.garmin.tcx+xml",
+    )
     mock_response.text = "<test>data</test>"
-    mock_response.headers = {"content-type": "application/vnd.garmin.tcx+xml"}
-    mock_response.status_code = 200
     mock_oauth_session.request.return_value = mock_response
 
     result = base_resource._make_request("test/endpoint")
     assert result == "<test>data</test>"
 
 
-def test_make_request_unexpected_content_type(base_resource, mock_oauth_session, mock_response):
+def test_make_request_unexpected_content_type(
+    base_resource, mock_oauth_session, mock_response_factory
+):
     """Test handling of unexpected content type"""
+    mock_response = mock_response_factory(
+        200, headers={"content-type": "text/plain"}, content_type="text/plain"
+    )
     mock_response.text = "some data"
-    mock_response.headers = {"content-type": "text/plain"}
-    mock_response.status_code = 200
     mock_oauth_session.request.return_value = mock_response
 
     result = base_resource._make_request("test/endpoint")