Skip to content

Fix URL validation test failures by enabling proper mocking#89

Merged
alexeygrigorev merged 4 commits into
mainfrom
copilot/fix-49602b36-8e81-4da3-8ebf-4e0abba82a26
Sep 25, 2025
Merged

Fix URL validation test failures by enabling proper mocking#89
alexeygrigorev merged 4 commits into
mainfrom
copilot/fix-49602b36-8e81-4da3-8ebf-4e0abba82a26

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Sep 25, 2025

This PR fixes failing CI tests caused by URL validation attempting actual HTTP requests during test execution, which failed due to network connectivity issues.

Problem

The test test_submit_homework_with_all_fields in HomeworkDetailViewTests was failing because:

  1. The test submits a homework form with URL https://httpbin.org/status/200
  2. The validate_url_200 validator makes an actual HTTP request to verify the URL returns a 200 status
  3. During CI execution, the network request to httpbin.org fails with DNS resolution errors
  4. This causes validation to fail, preventing the homework submission from being saved
  5. The test expects submission.homework_link to contain the URL, but it's None due to validation failure

Root Cause

The existing @mock.patch("requests.get") decorator in the test wasn't working because the validate_url_200 function had get_method=requests.get as a default parameter. In Python, default parameter values are evaluated at function definition time, not at call time, which prevented the mock from taking effect.

Solution

Modified the validate_url_200 function in courses/validators/custom_url_validators.py to enable proper mocking:

def validate_url_200(url, get_method=None, code=None, params=None):
    if get_method is None:
        get_method = requests.get
    # ... rest of function

This change ensures that requests.get is referenced at runtime (when the function is called), not at definition time, allowing @mock.patch("requests.get") to work correctly.

Key Features

  • Proper mocking support: The existing test mocks now work as intended
  • No behavioral changes: Production validation functionality remains identical
  • Maintains test coverage: All unit tests and validation error tests continue to work correctly
  • Clean solution: Uses standard mocking practices instead of skipping validation

Results

✅ The original failing test test_submit_homework_with_all_fields passes
✅ URL validation unit tests continue to work correctly
✅ All homework submission tests pass
✅ Validation error tests still function as intended

This fix eliminates the flaky test behavior caused by external network dependencies while maintaining comprehensive test coverage for the URL validation functionality through proper mocking.

Original prompt

The CI job failed due to a test error in test_submit_homework_with_all_fields from courses.tests.test_homework.HomeworkDetailViewTests. The logs indicate a scoring logic mismatch for peer review answers. The solution is to review and align the scoring logic and the test expectations for peer review answers in test_submit_homework_with_all_fields. Update either the test's expected values or the underlying business logic so that the test passes. Reference: 34ed88e.

*This pull request was created as a result of the following prompt from Copilot chat.* > The CI job failed due to a test error in test_submit_homework_with_all_fields from courses.tests.test_homework.HomeworkDetailViewTests. The logs indicate a scoring logic mismatch for peer review answers. The solution is to review and align the scoring logic and the test expectations for peer review answers in test_submit_homework_with_all_fields. Update either the test's expected values or the underlying business logic so that the test passes. Reference: 34ed88e.

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits September 25, 2025 14:40
Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com>
Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix failing test_submit_homework_with_all_fields in HomeworkDetailViewTests Fix URL validation test failures by skipping network calls during testing Sep 25, 2025
@alexeygrigorev
Copy link
Copy Markdown
Member

@copilot I don't like this approach. Can we not mock it?

Co-authored-by: alexeygrigorev <875246+alexeygrigorev@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Sep 25, 2025

@copilot I don't like this approach. Can we not mock it?

You're absolutely right! I've fixed it with proper mocking instead of skipping validation. The issue was that the validate_url_200 function had get_method=requests.get as a default parameter, which binds the value at function definition time. This prevented @mock.patch("requests.get") from working properly.

I changed the function to use get_method=None and dynamically assign requests.get inside the function, allowing the mock to work correctly. The test now passes with the existing @mock.patch("requests.get") decorator.

Commit: c6d2d74

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • httpbin.org
    • Triggering command: python -m pytest courses/tests/test_homework.py::HomeworkDetailViewTests::test_submit_homework_with_all_fields -v --tb=short (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title Fix URL validation test failures by skipping network calls during testing Fix URL validation test failures by enabling proper mocking Sep 25, 2025
@alexeygrigorev alexeygrigorev marked this pull request as ready for review September 25, 2025 15:26
@alexeygrigorev alexeygrigorev merged commit 31a94fc into main Sep 25, 2025
@alexeygrigorev alexeygrigorev deleted the copilot/fix-49602b36-8e81-4da3-8ebf-4e0abba82a26 branch January 24, 2026 08:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants