Skip to content

Fix failing tests#1535

Merged
ihalaij1 merged 2 commits into
apluslms:masterfrom
reppuli92:test_fix
Jul 24, 2026
Merged

Fix failing tests#1535
ihalaij1 merged 2 commits into
apluslms:masterfrom
reppuli92:test_fix

Conversation

@reppuli92

Copy link
Copy Markdown
Contributor

Description

What?

Several tests were failing in the a-plus test suite. Fix these issues

Why?

To improve testing functionality to catch possible future issues

How?

Edited test_uploading_and_viewing_file in exercise/tests.py to make the test assert the file uploading and viewing independently of feedback rendering.

Edited exercise/mixins.py to raise DRF NotFound instead of Django error message so that the tests can catch the correct error message.

FInally, edited get_queryset in api/views.py so that the sets can be searched with either search- or view-based filtering.

Testing

Remember to add or update unit tests for new features and changes.

What type of test did you run?

  • Accessibility test using the WAVE extension.
  • Django unit tests.
  • Playwright tests.
  • Other test. (Add a description below)
  • Manual testing.

In addition to verifying several times that the test suite now passes, I also briefly checked that the local Docker image also works.

Did you test the changes in

  • Chrome
  • Firefox
  • This pull request cannot be tested in the browser.

Think of what is affected by these changes and could become broken

Translation

Programming style

  • Did you follow our style guides?
  • Did you use Python type hinting in all functions that you added or edited? (type hints for function parameters and return values)

Have you updated the README or other relevant documentation?

  • documents inside the doc directory.
  • README.md.
  • Aplus Manual.
  • Other documentation (mention below which documentation).

Is it Done?

  • Reviewer has finished the code review
  • After the review, the developer has made changes accordingly
  • Customer/Teacher has accepted the implementation of the feature

Clean up your git commit history before submitting the pull request!

@ihalaij1
ihalaij1 requested a review from Copilot July 24, 2026 06:46
@ihalaij1 ihalaij1 self-assigned this Jul 24, 2026
@ihalaij1
ihalaij1 self-requested a review July 24, 2026 06:46
@ihalaij1 ihalaij1 moved this from Todo to Under review in A+ sprints Jul 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses failing tests by reducing external/service dependencies in exercise submission tests, aligning API exception types with DRF expectations, and expanding user list filtering to support a dedicated field/values filter in addition to search.

Changes:

  • Updated the users API list endpoint to allow FieldValuesFilter-based filtering (in addition to search) while still preventing unfiltered user listing.
  • Adjusted test_uploading_and_viewing_file to avoid exercising remote feedback-page loading when the test intent is file upload/view behavior.
  • Switched exercise API mixins to raise DRF NotFound instead of Django Http404 for consistent API error handling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
userprofile/api/views.py Adds FieldValuesFilter support and relaxes list restrictions when a field-based filter is provided.
exercise/tests.py Patches out feedback-page loading during upload/view test to isolate test intent and avoid external calls.
exercise/api/mixins.py Uses DRF NotFound exceptions for missing exercise/submission objects in API mixins.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread userprofile/api/views.py Outdated
Comment thread exercise/tests.py Outdated
Comment thread userprofile/api/views.py

@ihalaij1 ihalaij1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I made some changes.

Reverted the Http404 -> NotFound() change, since the other mixins like authorization/api/mixins.py and course/api/mixins.py also use Http404() and DRF converts it to a NotFound() automatically anyway.

I also made some fixes so that tracebacks from tests that are expected to raise exceptions do not pollute the test output.

All tests should now pass.

Exclude already-loaded modules from incremental ContentDBData fetches,
preserve CourseModule ordering after merging batches, and add regression
coverage for loading a module before its containing instance.
@ihalaij1
ihalaij1 merged commit 9c8752a into apluslms:master Jul 24, 2026
8 checks passed
@github-project-automation github-project-automation Bot moved this from Under review to Done in A+ sprints Jul 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants