Skip to content

Add integration tests for GSPro and E6 simulator protocols - #11

Open
atharva-matale wants to merge 1 commit into
r87-e:mainfrom
atharva-matale:jarvis-fix-1775375678
Open

Add integration tests for GSPro and E6 simulator protocols#11
atharva-matale wants to merge 1 commit into
r87-e:mainfrom
atharva-matale:jarvis-fix-1775375678

Conversation

@atharva-matale

Copy link
Copy Markdown

Closes #8

This PR adds integration tests to verify the complete communication cycle for the GSPro and E6 simulator protocols, covering shot sending, response validation, and heartbeat functionality. The tests establish connections, send simulated shots, and confirm the received data conforms to the expected protocol specifications, including handling potential disconnects and malformed input. These tests will help ensure the reliability and correctness of the simulator protocol implementations.

@r87-e r87-e left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Good start on the integration tests! A few issues to address:

Port 921 requires root

let listener = TcpListener::bind("127.0.0.1:921").await?;

Port 921 is privileged (< 1024) — this will fail on CI. Use port 0 to let the OS assign a random available port:

let listener = TcpListener::bind("127.0.0.1:0").await?;
let addr = listener.local_addr()?;

Race condition

The test connects to the server before calling accept(). The accept should be spawned first (or use tokio::spawn to run the server side concurrently), otherwise the connection may fail if the test runs faster than expected.

Type mismatch

The test deserializes the received data as GsProResponse, but send_shot() sends a GsProShotMessage. The server side should read the shot message, not a response.

Unused import

async_trait is imported but never used — remove it (and from Cargo.toml if it was added as a dependency).

Missing newline at end of file

The test structure is right — just needs these fixes. Would also be great to add an E6 protocol test while you're at it (that was part of the original issue #8).

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.

Add integration tests for GSPro and E6 protocols

2 participants