[tests] Add WebSocket consumer tests #677 by shivsubh · Pull Request #678 · openwisp/openwisp-radius

shivsubh · 2026-02-05T10:24:51Z

Implement tests for RadiusBatchConsumer to verify authentication, authorization, and group messaging logic.
Increases coverage for consumers.py to 84%.

Checklist

I have read the OpenWISP Contributing Guidelines.
I have manually tested the changes proposed in this pull request.
I have written new test cases for new code and/or updated existing tests for changes to existing code.
I have updated the documentation.

Reference to Existing Issue

Closes #677

coderabbitai · 2026-02-05T10:25:11Z

Walkthrough

Adds a comprehensive end-to-end test suite for RadiusBatchConsumer at openwisp_radius/tests/test_consumers.py. The new TestRadiusBatchConsumer configures ASGI routing for /ws/radius/batch/{batch_id}/ and verifies WebSocket connection and authorization across superusers, staff (with/without permissions), unauthenticated/non-staff users, and group-based access. It exercises batch status update messaging via the channel layer, disconnect cleanup, and the internal _user_can_access_batch access checks (including invalid UUID handling). Additionally, multiple unit tests were added under test_counters, test_sqlite_counters, test_radclient, and test_commands. consumers.py simplifies _user_can_access_batch for non-superusers to use a direct exists() query.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client (WebSocket)
    participant ASGI as ASGI Router (/ws/radius/batch/{batch_id}/)
    participant Consumer as RadiusBatchConsumer
    participant Auth as Auth System
    participant DB as Database (RadiusBatch)
    participant Channel as Channel Layer

    Client->>ASGI: Open WebSocket /ws/radius/batch/{batch_id}/
    ASGI->>Consumer: Instantiate consumer (scope, path)
    Consumer->>Auth: authenticate user (scope)
    Auth-->>Consumer: user (superuser/staff/group/anon)
    alt user is superuser
        Consumer->>DB: (optional) skip org check
        DB-->>Consumer: n/a
        Consumer-->>Client: accept connection
    else non-superuser
        Consumer->>DB: RadiusBatch.objects.filter(pk=batch_id, organization__in=user.organizations_managed).exists()
        DB-->>Consumer: True/False
        alt exists True
            Consumer-->>Client: accept connection
        else
            Consumer-->>Client: reject connection
        end
    end

    %% Status update flow
    Client->>Consumer: send batch.status.update payload
    Consumer->>Channel: group_send "batch_{id}" update payload
    Channel-->>Consumer: delivers update to group members
    Consumer-->>Client: forwards update payload over WebSocket

    %% Disconnect cleanup
    Client->>Consumer: close WebSocket
    Consumer->>Channel: group_discard "batch_{id}" (cleanup)
    Channel-->>Consumer: confirm discard

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding WebSocket consumer tests. It directly references the issue number and is appropriately concise.
Description check	✅ Passed	The pull request description follows the template structure with all required sections completed: checklist items marked, issue reference provided, and a clear description of the changes made.
Linked Issues check	✅ Passed	The PR adds comprehensive tests for RadiusBatchConsumer covering authentication, authorization, and WebSocket messaging, which directly addresses the test coverage improvement objective in issue `#677`.
Out of Scope Changes check	✅ Passed	All changes are within scope: test additions for RadiusBatchConsumer, a minor refactor in consumers.py to support testing, and related counter tests. No unrelated functionality was introduced.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 4

🤖 Fix all issues with AI agents

In @.github/workflows/ci.yml:
- Around line 102-107: Add a permissions block to the job granting
pull-requests: write, give the Tests step an id (e.g., id: tests) so downstream
steps can reference its conclusion, and update the "Comment coverage" step
(name: Comment coverage) to pin the action to
MishaKav/pytest-coverage-comment@v1.2.0 and change its condition to run only
when tests succeed (use steps.tests.conclusion == 'success' instead of the
current condition); keep the pytest-coverage-path input as-is.

In `@openwisp_radius/tests/test_consumers.py`:
- Around line 47-52: Replace hardcoded password literals passed to
User.objects.create_user in tests (e.g., the "staff_user" and other instances
created in test_consumers.py) with a shared constant or helper function; define
a PASSWORD constant (or a get_test_password() helper) at top of the test module
and use that constant when calling create_user so Ruff S106 is satisfied and all
occurrences (around the create_user calls at the referenced spots) are updated
to use the constant/helper.
- Around line 65-66: The test unpacking of the tuple returned by
_create_test_data() currently assigns values to names that go unused and
triggers RUF059; update each unpack to either unpack only the needed values or
rename unused names with a leading underscore (e.g., change "org, user, batch =
self._create_test_data()" to "_org, user, _batch = self._create_test_data()" or
similar) in all affected occurrences that call self._create_test_data()
(including the instances around lines 65, 82, 100, 118, 135, and 158) so unused
variables are prefixed with _ or omitted entirely.
- Around line 134-153: The test test_websocket_reject_no_permission fails
because RadiusBatchConsumer._user_can_access_batch currently discards the
queryset.exists() result and ends up returning True for non-superusers; update
the method in openwisp_radius/consumers.py
(RadiusBatchConsumer._user_can_access_batch) to return the actual boolean from
the permission check (e.g., return batch_qs.exists() or the equivalent
membership check) for non-superusers instead of always True, so staff users
without access are denied and the test expectation holds.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2646a5c and 0b0decd.

📒 Files selected for processing (2)

.github/workflows/ci.yml
openwisp_radius/tests/test_consumers.py

🧰 Additional context used

🧬 Code graph analysis (1)

openwisp_radius/tests/test_consumers.py (4)

openwisp_radius/consumers.py (3)

RadiusBatchConsumer (8-47)

connect (23-41)

disconnect (43-44)

openwisp_radius/utils.py (1)

load_model (33-34)

openwisp_radius/tests/__init__.py (2)

CreateRadiusObjectsMixin (53-150)

_create_radius_batch (137-142)

openwisp_radius/api/serializers.py (1)

user (673-674)

🪛 Ruff (0.14.14)

openwisp_radius/tests/test_consumers.py

[error] 50-50: Possible hardcoded password assigned to argument: "password"

(S106)

[error] 60-60: Possible hardcoded password assigned to argument: "password"

(S106)

[warning] 65-65: Unpacked variable org is never used