carverauto/serviceradar
1
1
Fork 0
Code Issues 190 Pull requests 16 Projects Releases 173 Packages Wiki Activity Actions

adding more tests for devices queries #2427

Merged
mfreeman451 merged 1 commit from refs/pull/2427/head into main 2025-11-19 17:26:27 +00:00
Conversation 2 Commits 1 Files changed 2 +304 -87
mfreeman451 commented 2025-11-19 17:14:12 +00:00 (Migrated from github.com)
Owner
Copy link

Imported from GitHub pull request.

Original GitHub pull request: #1959
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/1959
Original created: 2025-11-19T17:14:12Z
Original updated: 2025-11-19T17:26:54Z
Original head: carverauto/serviceradar:srql/queries_test
Original base: main
Original merged: 2025-11-19T17:26:27Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

  • I have signed the DCO?
  • The build completes without errors?
  • All tests are passing when running make test?

PR Type

Tests

Description

  • Add seven new device query test cases covering time filters

  • Test sorting by last_seen in ascending and descending order

  • Test limit functionality and is_available filter conditions

  • Add device-delta fixture with 8-day old timestamp for testing

  • Reformat SQL seed file with consistent indentation and spacing

Diagram Walkthrough

flowchart LR
  A["Test Cases"] -->|"time filters"| B["last_7d, last_1h"]
  A -->|"sorting"| C["last_seen asc/desc"]
  A -->|"filtering"| D["limit, is_available"]
  E["Seed Data"] -->|"new device"| F["device-delta"]
  E -->|"reformatted"| G["SQL consistency"]
  B --> H["Comprehensive Coverage"]
  C --> H
  D --> H
  F --> H

File Walkthrough
Relevant files
Tests
comprehensive_queries.rs
Add device query test cases with validators                           

rust/srql/tests/comprehensive_queries.rs

  • Add seven new test cases for device queries with detailed comments
  • Test time-based filtering with time:last_7d and time:last_1h
  • Test sorting functionality with sort:last_seen:desc and
    sort:last_seen:asc
  • Test limit and is_available filter conditions with result validation
 +64/-0   
seed.sql
Add device-delta fixture and reformat SQL                               

rust/srql/tests/fixtures/seed.sql

  • Add new device-delta fixture with 8-day old last_seen timestamp
  • Reformat entire SQL file with consistent indentation (4 spaces)
  • Update device-alpha last_seen from 1 hour to 30 minutes
  • Standardize SQL formatting across all INSERT statements and CTEs
 +240/-87
Imported from GitHub pull request. Original GitHub pull request: #1959 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/1959 Original created: 2025-11-19T17:14:12Z Original updated: 2025-11-19T17:26:54Z Original head: carverauto/serviceradar:srql/queries_test Original base: main Original merged: 2025-11-19T17:26:27Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Tests ___ ### **Description** - Add seven new device query test cases covering time filters - Test sorting by last_seen in ascending and descending order - Test limit functionality and is_available filter conditions - Add device-delta fixture with 8-day old timestamp for testing - Reformat SQL seed file with consistent indentation and spacing ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Test Cases"] -->|"time filters"| B["last_7d, last_1h"] A -->|"sorting"| C["last_seen asc/desc"] A -->|"filtering"| D["limit, is_available"] E["Seed Data"] -->|"new device"| F["device-delta"] E -->|"reformatted"| G["SQL consistency"] B --> H["Comprehensive Coverage"] C --> H D --> H F --> H ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>comprehensive_queries.rs</strong><dd><code>Add device query test cases with validators</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> rust/srql/tests/comprehensive_queries.rs <ul><li>Add seven new test cases for device queries with detailed comments<br> <li> Test time-based filtering with <code>time:last_7d</code> and <code>time:last_1h</code><br> <li> Test sorting functionality with <code>sort:last_seen:desc</code> and <br><code>sort:last_seen:asc</code><br> <li> Test limit and is_available filter conditions with result validation</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1959/files#diff-2d17debb1fb0e91ce3a1ec48f779dca4a145489187cd73b1d2848a6312d48c8c">+64/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td> <details> <summary><strong>seed.sql</strong><dd><code>Add device-delta fixture and reformat SQL</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> rust/srql/tests/fixtures/seed.sql <ul><li>Add new <code>device-delta</code> fixture with 8-day old last_seen timestamp<br> <li> Reformat entire SQL file with consistent indentation (4 spaces)<br> <li> Update <code>device-alpha</code> last_seen from 1 hour to 30 minutes<br> <li> Standardize SQL formatting across all INSERT statements and CTEs</ul> </details> </td> <td><a href="https://github.com/carverauto/serviceradar/pull/1959/files#diff-1de433f1e0e2be718b04d148299c8f2c8c46a1f9c0b452f55bc1067e82b4edc1">+240/-87</a></td> </tr> </table></td></tr></tr></tbody></table> </details> ___
qodo-code-review[bot] commented 2025-11-19 17:14:48 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1959#issuecomment-3553826780
Original created: 2025-11-19T17:14:48Z

PR Compliance Guide 🔍

(Compliance updated until commit github.com/carverauto/serviceradar@64e366a4f7)

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

⚪
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Not applicable tests: The PR adds test cases and seed data without introducing or omitting any production audit
logging; applicability to audit trails cannot be determined from test-only changes.

Referred Code 
// Device Query Tests
TestCase {
    // device-delta is 8 days old, so last_7d should exclude it.
    // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included.
    query: "in:devices time:last_7d",
    expected_count: 3,
    validator: None,
},
TestCase {
    // Only device-alpha (30m) is within the last hour.
    query: "in:devices time:last_1h",
    expected_count: 1,
    validator: Some(Box::new(|body| {
        assert_eq!(body["results"][0]["device_id"], "device-alpha")
    })),
},
TestCase {
    // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d)
    query: "in:devices sort:last_seen:desc",
    expected_count: 4,
    validator: Some(Box::new(|body| {


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
No error paths: The added code is test assertions and SQL fixtures without runtime error handling changes,
so robustness of error handling in application code cannot be assessed from this diff.

Referred Code 
// Device Query Tests
TestCase {
    // device-delta is 8 days old, so last_7d should exclude it.
    // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included.
    query: "in:devices time:last_7d",
    expected_count: 3,
    validator: None,
},
TestCase {
    // Only device-alpha (30m) is within the last hour.
    query: "in:devices time:last_1h",
    expected_count: 1,
    validator: Some(Box::new(|body| {
        assert_eq!(body["results"][0]["device_id"], "device-alpha")
    })),
},
TestCase {
    // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d)
    query: "in:devices sort:last_seen:desc",
    expected_count: 4,
    validator: Some(Box::new(|body| {


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Test-only changes: No user-facing error handling is introduced or modified in these test additions; security
of error messages cannot be evaluated here.

Referred Code 
// Device Query Tests
TestCase {
    // device-delta is 8 days old, so last_7d should exclude it.
    // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included.
    query: "in:devices time:last_7d",
    expected_count: 3,
    validator: None,
},
TestCase {
    // Only device-alpha (30m) is within the last hour.
    query: "in:devices time:last_1h",
    expected_count: 1,
    validator: Some(Box::new(|body| {
        assert_eq!(body["results"][0]["device_id"], "device-alpha")
    })),
},
TestCase {
    // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d)
    query: "in:devices sort:last_seen:desc",
    expected_count: 4,
    validator: Some(Box::new(|body| {


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Fixture content: The SQL seed inserts synthetic device and trace data including IPs and identifiers for
testing; since no runtime logging behavior is added or modified, secure logging compliance
cannot be determined from this diff.

Referred Code 
-- Canonical SRQL device fixture rows.
TRUNCATE unified_devices RESTART IDENTITY;
WITH base AS (
    SELECT NOW() AS now_ts
)
INSERT INTO unified_devices (
        device_id,
        ip,
        poller_id,
        agent_id,
        hostname,
        mac,
        discovery_sources,
        is_available,
        first_seen,
        last_seen,
        metadata,
        device_type,
        service_type,
        service_status,
        last_heartbeat,


 ... (clipped 306 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Query strings: The PR adds test query strings and seed data only; it does not modify input validation or
query parameterization in production code, so security of input handling cannot be
assessed from this diff.

Referred Code 
// Device Query Tests
TestCase {
    // device-delta is 8 days old, so last_7d should exclude it.
    // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included.
    query: "in:devices time:last_7d",
    expected_count: 3,
    validator: None,
},
TestCase {
    // Only device-alpha (30m) is within the last hour.
    query: "in:devices time:last_1h",
    expected_count: 1,
    validator: Some(Box::new(|body| {
        assert_eq!(body["results"][0]["device_id"], "device-alpha")
    })),
},
TestCase {
    // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d)
    query: "in:devices sort:last_seen:desc",
    expected_count: 4,
    validator: Some(Box::new(|body| {


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

Previous compliance checks

Compliance check up to commit 64e366a
Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
⚪
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

⚪
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Test Coverage: The PR adds only tests and fixtures and does not introduce application logic that would
affect audit trails; verifying comprehensive audit logging is outside the scope of the
shown changes.

Referred Code 
// Device Query Tests
TestCase {
    // device-delta is 8 days old, so last_7d should exclude it.
    // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included.
    query: "in:devices time:last_7d",
    expected_count: 3,
    validator: None,
},
TestCase {
    // Only device-alpha (30m) is within the last hour.
    query: "in:devices time:last_1h",
    expected_count: 1,
    validator: Some(Box::new(|body| {
        assert_eq!(body["results"][0]["device_id"], "device-alpha")
    })),
},
TestCase {
    // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d)
    query: "in:devices sort:last_seen:desc",
    expected_count: 4,
    validator: Some(Box::new(|body| {


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Test Assertions: New test code asserts expected results but does not include explicit error handling paths;
robustness of runtime error handling cannot be assessed from these additions alone.

Referred Code 
// Device Query Tests
TestCase {
    // device-delta is 8 days old, so last_7d should exclude it.
    // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included.
    query: "in:devices time:last_7d",
    expected_count: 3,
    validator: None,
},
TestCase {
    // Only device-alpha (30m) is within the last hour.
    query: "in:devices time:last_1h",
    expected_count: 1,
    validator: Some(Box::new(|body| {
        assert_eq!(body["results"][0]["device_id"], "device-alpha")
    })),
},
TestCase {
    // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d)
    query: "in:devices sort:last_seen:desc",
    expected_count: 4,
    validator: Some(Box::new(|body| {


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Log Data Content: Seed data inserts log rows with generic messages but assessing actual logging practices
and potential sensitive data exposure in runtime logs is not possible from these fixture
changes alone.

Referred Code 
        timestamp,
        trace_id,
        span_id,
        severity_text,
        severity_number,
        body,
        service_name,
        service_version,
        service_instance,
        scope_name,
        scope_version,
        attributes,
        resource_attributes,
        created_at
    )
SELECT base.now_ts - INTERVAL '1 minute',
    'trace-1',
    'span-1',
    'INFO',
    9,
    'Application started',


 ... (clipped 25 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Input Validation: Added tests construct SRQL query strings but do not demonstrate input validation or
sanitization in the application layer; security of input handling cannot be determined
from the test additions.

Referred Code 
// Device Query Tests
TestCase {
    // device-delta is 8 days old, so last_7d should exclude it.
    // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included.
    query: "in:devices time:last_7d",
    expected_count: 3,
    validator: None,
},
TestCase {
    // Only device-alpha (30m) is within the last hour.
    query: "in:devices time:last_1h",
    expected_count: 1,
    validator: Some(Box::new(|body| {
        assert_eq!(body["results"][0]["device_id"], "device-alpha")
    })),
},
TestCase {
    // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d)
    query: "in:devices sort:last_seen:desc",
    expected_count: 4,
    validator: Some(Box::new(|body| {


 ... (clipped 44 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1959#issuecomment-3553826780 Original created: 2025-11-19T17:14:48Z --- ## PR Compliance Guide 🔍 <!-- https://github.com/carverauto/serviceradar/commit/64e366a4f744a1d65334caef628027a203a72ea7 --> #### (Compliance updated until commit https://github.com/carverauto/serviceradar/commit/64e366a4f744a1d65334caef628027a203a72ea7) Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </strong></summary> - [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true --> </details></td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=1>🟢</td><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=5>⚪</td> <td><details> <summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1959/files#diff-2d17debb1fb0e91ce3a1ec48f779dca4a145489187cd73b1d2848a6312d48c8cR50-R114'><strong>Not applicable tests</strong></a>: The PR adds test cases and seed data without introducing or omitting any production audit <br>logging; applicability to audit trails cannot be determined from test-only changes.<br> <details open><summary>Referred Code</summary> ```rust // Device Query Tests TestCase { // device-delta is 8 days old, so last_7d should exclude it. // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included. query: "in:devices time:last_7d", expected_count: 3, validator: None, }, TestCase { // Only device-alpha (30m) is within the last hour. query: "in:devices time:last_1h", expected_count: 1, validator: Some(Box::new(|body| { assert_eq!(body["results"][0]["device_id"], "device-alpha") })), }, TestCase { // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d) query: "in:devices sort:last_seen:desc", expected_count: 4, validator: Some(Box::new(|body| { ... (clipped 44 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1959/files#diff-2d17debb1fb0e91ce3a1ec48f779dca4a145489187cd73b1d2848a6312d48c8cR50-R114'><strong>No error paths</strong></a>: The added code is test assertions and SQL fixtures without runtime error handling changes, <br>so robustness of error handling in application code cannot be assessed from this diff.<br> <details open><summary>Referred Code</summary> ```rust // Device Query Tests TestCase { // device-delta is 8 days old, so last_7d should exclude it. // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included. query: "in:devices time:last_7d", expected_count: 3, validator: None, }, TestCase { // Only device-alpha (30m) is within the last hour. query: "in:devices time:last_1h", expected_count: 1, validator: Some(Box::new(|body| { assert_eq!(body["results"][0]["device_id"], "device-alpha") })), }, TestCase { // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d) query: "in:devices sort:last_seen:desc", expected_count: 4, validator: Some(Box::new(|body| { ... (clipped 44 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1959/files#diff-2d17debb1fb0e91ce3a1ec48f779dca4a145489187cd73b1d2848a6312d48c8cR50-R114'><strong>Test-only changes</strong></a>: No user-facing error handling is introduced or modified in these test additions; security <br>of error messages cannot be evaluated here.<br> <details open><summary>Referred Code</summary> ```rust // Device Query Tests TestCase { // device-delta is 8 days old, so last_7d should exclude it. // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included. query: "in:devices time:last_7d", expected_count: 3, validator: None, }, TestCase { // Only device-alpha (30m) is within the last hour. query: "in:devices time:last_1h", expected_count: 1, validator: Some(Box::new(|body| { assert_eq!(body["results"][0]["device_id"], "device-alpha") })), }, TestCase { // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d) query: "in:devices sort:last_seen:desc", expected_count: 4, validator: Some(Box::new(|body| { ... (clipped 44 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1959/files#diff-1de433f1e0e2be718b04d148299c8f2c8c46a1f9c0b452f55bc1067e82b4edc1R1-R327'><strong>Fixture content</strong></a>: The SQL seed inserts synthetic device and trace data including IPs and identifiers for <br>testing; since no runtime logging behavior is added or modified, secure logging compliance <br>cannot be determined from this diff.<br> <details open><summary>Referred Code</summary> ```sql -- Canonical SRQL device fixture rows. TRUNCATE unified_devices RESTART IDENTITY; WITH base AS ( SELECT NOW() AS now_ts ) INSERT INTO unified_devices ( device_id, ip, poller_id, agent_id, hostname, mac, discovery_sources, is_available, first_seen, last_seen, metadata, device_type, service_type, service_status, last_heartbeat, ... (clipped 306 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1959/files#diff-2d17debb1fb0e91ce3a1ec48f779dca4a145489187cd73b1d2848a6312d48c8cR50-R114'><strong>Query strings</strong></a>: The PR adds test query strings and seed data only; it does not modify input validation or <br>query parameterization in production code, so security of input handling cannot be <br>assessed from this diff.<br> <details open><summary>Referred Code</summary> ```rust // Device Query Tests TestCase { // device-delta is 8 days old, so last_7d should exclude it. // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included. query: "in:devices time:last_7d", expected_count: 3, validator: None, }, TestCase { // Only device-alpha (30m) is within the last hour. query: "in:devices time:last_1h", expected_count: 1, validator: Some(Box::new(|body| { assert_eq!(body["results"][0]["device_id"], "device-alpha") })), }, TestCase { // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d) query: "in:devices sort:last_seen:desc", expected_count: 4, validator: Some(Box::new(|body| { ... (clipped 44 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> <!-- placeholder --> <!-- /compliance --update_compliance=true --> </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details> ___ #### Previous compliance checks <details> <summary>Compliance check up to commit <a href='https://github.com/carverauto/serviceradar/commit/64e366a4f744a1d65334caef628027a203a72ea7'>64e366a</a></summary><br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td>🟢</td><td><details><summary><strong>No security concerns identified</strong></summary> No security vulnerabilities detected by AI analysis. Human verification advised for critical code. </details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary>🎫 <strong>No ticket provided </strong></summary> - [ ] Create ticket/issue <!-- /create_ticket --create_ticket=true --> </details></td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=2>🟢</td><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=4>⚪</td> <td><details> <summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1959/files#diff-2d17debb1fb0e91ce3a1ec48f779dca4a145489187cd73b1d2848a6312d48c8cR50-R114'><strong>Test Coverage</strong></a>: The PR adds only tests and fixtures and does not introduce application logic that would <br>affect audit trails; verifying comprehensive audit logging is outside the scope of the <br>shown changes.<br> <details open><summary>Referred Code</summary> ```rust // Device Query Tests TestCase { // device-delta is 8 days old, so last_7d should exclude it. // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included. query: "in:devices time:last_7d", expected_count: 3, validator: None, }, TestCase { // Only device-alpha (30m) is within the last hour. query: "in:devices time:last_1h", expected_count: 1, validator: Some(Box::new(|body| { assert_eq!(body["results"][0]["device_id"], "device-alpha") })), }, TestCase { // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d) query: "in:devices sort:last_seen:desc", expected_count: 4, validator: Some(Box::new(|body| { ... (clipped 44 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1959/files#diff-2d17debb1fb0e91ce3a1ec48f779dca4a145489187cd73b1d2848a6312d48c8cR50-R114'><strong>Test Assertions</strong></a>: New test code asserts expected results but does not include explicit error handling paths; <br>robustness of runtime error handling cannot be assessed from these additions alone.<br> <details open><summary>Referred Code</summary> ```rust // Device Query Tests TestCase { // device-delta is 8 days old, so last_7d should exclude it. // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included. query: "in:devices time:last_7d", expected_count: 3, validator: None, }, TestCase { // Only device-alpha (30m) is within the last hour. query: "in:devices time:last_1h", expected_count: 1, validator: Some(Box::new(|body| { assert_eq!(body["results"][0]["device_id"], "device-alpha") })), }, TestCase { // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d) query: "in:devices sort:last_seen:desc", expected_count: 4, validator: Some(Box::new(|body| { ... (clipped 44 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1959/files#diff-1de433f1e0e2be718b04d148299c8f2c8c46a1f9c0b452f55bc1067e82b4edc1R236-R281'><strong>Log Data Content</strong></a>: Seed data inserts log rows with generic messages but assessing actual logging practices <br>and potential sensitive data exposure in runtime logs is not possible from these fixture <br>changes alone.<br> <details open><summary>Referred Code</summary> ```sql timestamp, trace_id, span_id, severity_text, severity_number, body, service_name, service_version, service_instance, scope_name, scope_version, attributes, resource_attributes, created_at ) SELECT base.now_ts - INTERVAL '1 minute', 'trace-1', 'span-1', 'INFO', 9, 'Application started', ... (clipped 25 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/1959/files#diff-2d17debb1fb0e91ce3a1ec48f779dca4a145489187cd73b1d2848a6312d48c8cR50-R114'><strong>Input Validation</strong></a>: Added tests construct SRQL query strings but do not demonstrate input validation or <br>sanitization in the application layer; security of input handling cannot be determined <br>from the test additions.<br> <details open><summary>Referred Code</summary> ```rust // Device Query Tests TestCase { // device-delta is 8 days old, so last_7d should exclude it. // device-alpha (30m), device-beta (3h), device-gamma (2h) should be included. query: "in:devices time:last_7d", expected_count: 3, validator: None, }, TestCase { // Only device-alpha (30m) is within the last hour. query: "in:devices time:last_1h", expected_count: 1, validator: Some(Box::new(|body| { assert_eq!(body["results"][0]["device_id"], "device-alpha") })), }, TestCase { // Sort by last_seen desc. device-alpha (30m) > gamma (2h) > beta (3h) > delta (8d) query: "in:devices sort:last_seen:desc", expected_count: 4, validator: Some(Box::new(|body| { ... (clipped 44 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> <!-- placeholder --> <!-- /compliance --update_compliance=true --> </td></tr></tbody></table> </details>
qodo-code-review[bot] commented 2025-11-19 17:16:12 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/1959#issuecomment-3553831969
Original created: 2025-11-19T17:16:12Z

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Use expect() instead of unwrap()

Replace unwrap() with expect() when accessing the results array in tests to
provide a more descriptive error message upon failure.

rust/srql/tests/comprehensive_queries.rs [71]

-let results = body["results"].as_array().unwrap();
+let results = body["results"]
+    .as_array()
+    .expect("`results` field should be an array");
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly recommends replacing unwrap() with expect() to provide more informative error messages in tests, which improves debuggability.

Low
  • More
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/1959#issuecomment-3553831969 Original created: 2025-11-19T17:16:12Z --- ## PR Code Suggestions ✨ <!-- 64e366a --> Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>General</td> <td> <details><summary>Use <code>expect()</code> instead of <code>unwrap()</code></summary> ___ **Replace <code>unwrap()</code> with <code>expect()</code> when accessing the <code>results</code> array in tests to <br>provide a more descriptive error message upon failure.** [rust/srql/tests/comprehensive_queries.rs [71]](https://github.com/carverauto/serviceradar/pull/1959/files#diff-2d17debb1fb0e91ce3a1ec48f779dca4a145489187cd73b1d2848a6312d48c8cR71-R71) ```diff -let results = body["results"].as_array().unwrap(); +let results = body["results"] + .as_array() + .expect("`results` field should be an array"); ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=0 --> <details><summary>Suggestion importance[1-10]: 4</summary> __ Why: The suggestion correctly recommends replacing `unwrap()` with `expect()` to provide more informative error messages in tests, which improves debuggability. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] More <!-- /improve --more_suggestions=true --> </td><td></td></tr></tbody></table>
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
1week

   
2weeks

   
Failed compliance check

   
IP cameras

   
NATS

NATS JetStream

  
Possible security concern

   
Review effort 1/5

   
Review effort 2/5

   
Review effort 3/5

   
Review effort 4/5

   
Review effort 5/5

   
UI

   
aardvark

   
accessibility

   
amd64

   
api

   
arm64

   
auth

   
back-end

   
bgp

   
blog

   
bug

Something isn't working

  
build

   
checkers

   
ci-cd

continuous integration-continuous deployments

  
cleanup

   
cnpg

cloud-native postgres

  
codex

   
core

core service

  
dependencies

Pull requests that update a dependency file

  
device-management

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
dusk

   
ebpf

   
enhancement

New feature or request

  
eta 1d

   
eta 1hr

   
eta 3d

   
eta 3hr

   
feature

   
fieldsurvey

   
github_actions

Pull requests that update GitHub Actions code

  
go

Pull requests that update Go code

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
javascript

Pull requests that update Javascript code

  
k8s

   
log-collector

   
mapper

   
mtr

multi traceroute

  
needs-triage

   
netflow

   
network-sweep

   
observability

   
oracle

Oracle Linux related issues

  
otel

opentelemetry logs, traces, metrics

  
plug-in

   
proton

timeplus proton streaming database

  
python

   
question

Further information is requested

  
reddit

   
redhat

   
research

   
rperf

   
rperf-checker

   
rust

Pull requests that update rust code

  
sdk

   
security

   
serviceradar-agent

   
serviceradar-agent-gateway

   
serviceradar-web

   
serviceradar-web-ng

   
siem

   
snmp

   
sysmon

   
topology

   
ubiquiti

   
wasm

   
wontfix

This will not be worked on

  
zen-engine
No labels
1week
2weeks
Failed compliance check
IP cameras
NATS
Possible security concern
Review effort 1/5
Review effort 2/5
Review effort 3/5
Review effort 4/5
Review effort 5/5
UI
aardvark
accessibility
amd64
api
arm64
auth
back-end
bgp
blog
bug
build
checkers
ci-cd
cleanup
cnpg
codex
core
dependencies
device-management
documentation
duplicate
dusk
ebpf
enhancement
eta 1d
eta 1hr
eta 3d
eta 3hr
feature
fieldsurvey
github_actions
go
good first issue
help wanted
invalid
javascript
k8s
log-collector
mapper
mtr
needs-triage
netflow
network-sweep
observability
oracle
otel
plug-in
proton
python
question
reddit
redhat
research
rperf
rperf-checker
rust
sdk
security
serviceradar-agent
serviceradar-agent-gateway
serviceradar-web
serviceradar-web-ng
siem
snmp
sysmon
topology
ubiquiti
wasm
wontfix
zen-engine
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!2427
No description provided.