Skip to content

Conversation

@kevinelliott
Copy link
Contributor

@kevinelliott kevinelliott commented Aug 13, 2025

Add Missing Tests

Summary

This PR adds comprehensive test coverage to previously untested or low-coverage sections across three Airframes repositories:

  • acars-decoder-typescript: Added tests for Label_SQ (13.33% → 90.83% coverage) and Label_20_POS (31.74% → 82.53% coverage) plugins

The tests focus on core parsing logic, validation edge cases, and error handling scenarios that were previously untested.

Review & Testing Checklist for Human

  • Validate ARINC message format accuracy - The Label_SQ test constructs specific ARINC ground station squitter messages; verify these match real-world message formats

Diagram

%%{ init : { "theme" : "default" }}%%
graph TB
    subgraph "acars-decoder-typescript"
        LabelSQ["lib/plugins/Label_SQ.ts"]:::context
        LabelSQTest["lib/plugins/Label_SQ.test.ts"]:::major-edit
        Label20POS["lib/plugins/Label_20_POS.ts"]:::context  
        Label20POSTest["lib/plugins/Label_20_POS.test.ts"]:::major-edit
        MessageDecoder["lib/MessageDecoder.ts"]:::context
    end

    LabelSQTest -->|"tests decode logic"| LabelSQ
    Label20POSTest -->|"tests position parsing"| Label20POS
    TestMain -->|"mocks dependencies"| AppMain

    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit  
        L3[Context/No Edit]:::context
    end

    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB
    classDef context fill:#FFFFFF
Loading

Notes

  • Coverage improvements: Label_SQ went from 13.33% to 90.83% coverage, Label_20_POS from 31.74% to 82.53%
  • Test approach: Used existing test patterns and mocking strategies from each codebase to maintain consistency

Session requested by: Kevin Elliott (@kevinelliott)
Devin session: https://app.devin.ai/sessions/144ecd3792924403b4cd3c823b89d108

Summary by CodeRabbit

  • Tests
    • Added unit tests for Label_20_POS: verify decoding of 11-field and 5-field POS messages with coordinates; ensure unknown variations are not decoded.
    • Added unit tests for Label_SQ: verify decoding of version 2 ARINC squitter with coordinates and frequency; confirm SITA network mapping with partial data; handle non–version-2 payloads.

Co-Authored-By: Kevin Elliott <kevin@welikeinc.com>
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@jazzberry-ai
Copy link

jazzberry-ai bot commented Aug 13, 2025

Bug Report

Name Severity Example test case Description
Lack of input validation in Label_SQ Medium V2AA01XA... with invalid chars The Label_SQ plugin does not thoroughly validate input data, potentially leading to incorrect parsing if the input contains unexpected characters in the ground station information. For example, invalid characters in the ground station ID or coordinates are not properly handled.

Comments? Email us.

@coderabbitai
Copy link

coderabbitai bot commented Aug 13, 2025

Walkthrough

Adds two new unit test files for plugins Label_20_POS and Label_SQ that exercise decoding across multiple input variants, asserting decode results, decodeLevel, raw fields, and formatted items. No production code or public API changes.

Changes

Cohort / File(s) Summary
Label_20_POS tests
lib/plugins/Label_20_POS.test.ts
New tests for Label_20_POS: verifies decoding of POS with 11-field (coordinates + metadata) and 5-field (coordinates only) variations, and verifies unknown variation is not decoded; checks decode flags, raw.position lat/long, formatted description/items.
Label_SQ tests
lib/plugins/Label_SQ.test.ts
New tests for Label_SQ: verifies parsing of v2 ARINC ground-station squitter (network, version, station codes, coordinates, VDL frequency), maps SITA network case, and handles non-v2 payloads; asserts decoded fields and formatted items.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

Suggested reviewers

  • fredclausen

Poem

A whisker twitch, a test well spun,
Two new hops beneath the sun.
POS pins points, SQ sings codes,
I nibble logs on testing roads.
Carrots counted, green lights won. 🥕


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d3270d5 and b7d6876.

📒 Files selected for processing (1)
  • lib/plugins/Label_20_POS.test.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • lib/plugins/Label_20_POS.test.ts
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch devin/1755126141-add-missing-tests-acars-decoder

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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@kevinelliott kevinelliott requested a review from makrsmark August 13, 2025 23:06
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (8)
lib/plugins/Label_20_POS.test.ts (4)

8-10: Pass a fully shaped Message object to satisfy typing and future-proof tests

Calling plugin.decode with only { text } relies on Message having text as the only required property. To avoid brittle tests if Message tightens in the future and to better mirror real pipeline input, include label: '20'.

-  const decode = (text: string) => {
-    return plugin.decode({ text });
-  };
+  const decode = (text: string) => {
+    return plugin.decode({ label: '20', text });
+  };

16-20: Also assert decoder identity for stronger verification

Verifying the decoder name guards against accidental routing to the wrong plugin if multiple handlers ever overlap.

     expect(res.decoded).toBe(true);
     expect(res.decoder.decodeLevel).toBe('full');
+    expect(res.decoder.name).toBe('label-20-pos');
     expect(res.formatted.description).toBe('Position Report');
     expect(res.raw.preamble).toBe('POS');

22-29: Validate preamble in the 5-field variation as well

This mirrors the 11-field test and ensures the plugin consistently sets the preamble.

     expect(res.decoded).toBe(true);
     expect(res.decoder.decodeLevel).toBe('full');
     expect(res.formatted.description).toBe('Position Report');
+    expect(res.raw.preamble).toBe('POS');

31-38: Optional: Add an integration test via MessageDecoder for end-to-end behavior

Since MessageDecoder filters by plugin qualifiers (labels/preambles), a small integration check would validate routing beyond direct plugin calls. Not required, but useful for regression safety.

I can draft an additional test using decoder.decode({ label: '20', text }) if you’d like.

lib/plugins/Label_SQ.test.ts (4)

8-10: Pass a fully shaped Message object to satisfy typing and future-proof tests

Include label: 'SQ' to align with the Message shape and real decoding flow.

-  const decode = (text: string) => {
-    return plugin.decode({ text });
-  };
+  const decode = (text: string) => {
+    return plugin.decode({ label: 'SQ', text });
+  };

12-41: Add assertion for formatted description

This ensures human-readable formatting remains stable.

     expect(res.decoder.decodeLevel).toBe('full');
 
+    expect(res.formatted.description).toBe('Ground Station Squitter');
     expect(res.raw.preamble).toBe(text.substring(0, 4));
     expect(res.raw.version).toBe('2');
     expect(res.raw.network).toBe('A');

43-57: Also assert decoder identity/decodeLevel for the SITA/no-match case

Even without a regex match, the plugin intentionally sets decoded=true and decodeLevel='full'. Asserting these protects behavior from future changes.

     const res = decode(text);
 
     expect(res.decoded).toBe(true);
+    expect(res.decoder.name).toBe('label-sq');
+    expect(res.decoder.decodeLevel).toBe('full');
     expect(res.raw.version).toBe('2');
     expect(res.raw.network).toBe('S');

59-70: Optionally assert description for non-v2 payloads

Keeps formatted output expectations uniform across versions.

     const res = decode(text);
 
     expect(res.decoded).toBe(true);
+    expect(res.formatted.description).toBe('Ground Station Squitter');
     expect(res.raw.version).toBe('1');
     expect(res.raw.network).toBe('A');
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2fc0e4d and d3270d5.

📒 Files selected for processing (2)
  • lib/plugins/Label_20_POS.test.ts (1 hunks)
  • lib/plugins/Label_SQ.test.ts (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
lib/plugins/Label_20_POS.test.ts (2)
lib/MessageDecoder.ts (1)
  • MessageDecoder (6-175)
lib/plugins/Label_20_POS.ts (1)
  • Label_20_POS (7-61)
lib/plugins/Label_SQ.test.ts (2)
lib/MessageDecoder.ts (1)
  • MessageDecoder (6-175)
lib/plugins/Label_SQ.ts (1)
  • Label_SQ (4-118)
🔇 Additional comments (1)
lib/plugins/Label_SQ.test.ts (1)

12-41: LGTM on core decoding assertions

Good coverage of version/network parsing, ground station extraction, coordinates, frequency, and formatted items.

…ted POS item

Co-Authored-By: Kevin Elliott <kevin@welikeinc.com>
@jazzberry-ai
Copy link

jazzberry-ai bot commented Aug 14, 2025

Bug Report

Name Severity Example test case Description
Label_20_POS: Missing Coordinate Format Tests Medium POS38160N077075W,,... The tests don't cover all possible coordinate formats, leading to potential parsing errors. For example, tests are missing for formats like 38160N077075W or 38.160N077.075W.
Label_20_POS: Missing Coordinate Validation Medium POSN91000W181000,,... There are no checks for valid latitude and longitude ranges (-90 to 90 and -180 to 180, respectively), which could lead to incorrect position data.
Label_20_POS: Silent Error Handling Low POSINVALID,,... Errors during coordinate parsing are handled silently, making debugging difficult. A warning or more informative error message should be logged.
Label_20_POS: Inconsistent decoding Low POSUNKNOWN When the plugin cannot decode a message, the function still sets decodeResult.formatted.description = 'Position Report'. This is inconsistent, as the message has not been decoded.
Label_SQ: ReDoS Vulnerability High V2AA01XAJFKKJFK34075N7398WV136975AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA The regex used to parse version 2 messages is vulnerable to Regular Expression Denial of Service (ReDoS). An attacker could craft a malicious input that causes the regex engine to consume excessive resources, leading to a denial of service.
Label_SQ: Limited Network Mapping Low V2XX... The network mapping only covers ARINC and SITA. Other networks might exist, and the plugin would default to 'Unknown.' This is fine, but it should be documented.
Label_SQ: Missing Unit Test for Coordinate Format Low N/A There's no unit test to confirm the coordinate format for ground stations, potentially leading to parsing issues if the format changes.

Comments? Email us.

@kevinelliott kevinelliott requested a review from makrsmark August 14, 2025 01:51
@kevinelliott kevinelliott merged commit 121794c into master Aug 14, 2025
9 checks passed
@kevinelliott kevinelliott deleted the devin/1755126141-add-missing-tests-acars-decoder branch August 14, 2025 02:02
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.

3 participants