diff --git a/backend/modules/api/tests/ws_integration_test.rs b/backend/modules/api/tests/ws_integration_test.rs index bd1bc50..8e6dad4 100644 --- a/backend/modules/api/tests/ws_integration_test.rs +++ b/backend/modules/api/tests/ws_integration_test.rs @@ -46,7 +46,7 @@ async fn connects_with_a_valid_token_and_receives_a_simulated_chess_game() { let game_id = "integration-game-1".to_string(); let token = make_access_token(42, "magnus"); - let url = format!("{}/v1/ws/game/{}", srv.url(""), game_id).replace("http://", "ws://"); + let url = srv.url(&format!("/v1/ws/game/{}", game_id)).replace("http://", "ws://"); let (_resp, mut connection) = awc::Client::new() .ws(url) @@ -117,7 +117,7 @@ async fn connects_with_a_valid_token_and_receives_a_simulated_chess_game() { #[actix_web::test] async fn rejects_connection_with_no_authorization_header() { let (srv, _lobby) = start_ws_test_server(); - let url = format!("{}/v1/ws/game/{}", srv.url(""), "game-no-auth").replace("http://", "ws://"); + let url = srv.url("/v1/ws/game/game-no-auth").replace("http://", "ws://"); let result = awc::Client::new().ws(url).connect().await; @@ -127,7 +127,7 @@ async fn rejects_connection_with_no_authorization_header() { #[actix_web::test] async fn rejects_connection_with_an_invalid_token() { let (srv, _lobby) = start_ws_test_server(); - let url = format!("{}/v1/ws/game/{}", srv.url(""), "game-bad-auth").replace("http://", "ws://"); + let url = srv.url("/v1/ws/game/game-bad-auth").replace("http://", "ws://"); let result = awc::Client::new() .ws(url) @@ -142,9 +142,7 @@ async fn rejects_connection_with_an_invalid_token() { async fn two_clients_in_the_same_game_both_receive_broadcasts() { let (srv, lobby) = start_ws_test_server(); let game_id = "integration-game-2".to_string(); - let base = srv.url(""); - - let url_a = format!("{base}/v1/ws/game/{game_id}").replace("http://", "ws://"); + let url_a = srv.url(&format!("/v1/ws/game/{game_id}")).replace("http://", "ws://"); let url_b = url_a.clone(); let (_r1, mut client_a) = awc::Client::new() @@ -181,7 +179,7 @@ async fn two_clients_in_the_same_game_both_receive_broadcasts() { async fn client_ping_is_answered_with_pong() { let (srv, _lobby) = start_ws_test_server(); let game_id = "integration-game-ping".to_string(); - let url = format!("{}/v1/ws/game/{}", srv.url(""), game_id).replace("http://", "ws://"); + let url = srv.url(&format!("/v1/ws/game/{}", game_id)).replace("http://", "ws://"); let token = make_access_token(7, "heartbeat_test"); let (_resp, mut connection) = awc::Client::new() @@ -200,21 +198,17 @@ async fn client_ping_is_answered_with_pong() { } } -/// Documents a real, current gap found while writing these tests (not -/// asserted as a bug fix — flagged here so it isn't silently relied upon): -/// `WsSession`'s `StreamHandler` for `ws::Message::Text` parses an incoming -/// client message into a `WsMessage` but its match arm body is empty — a -/// client sending a `Move` over the socket produces no broadcast, no -/// validation, and no response today. All moves in this test suite are -/// therefore simulated via the `Broadcast` actor message directly (as real -/// move-processing logic elsewhere in the app presumably does), not by -/// sending a `Move` WsMessage from the client. See PR description for the -/// recommended follow-up. +/// A `Move` sent by the client over the socket is parsed and broadcast to +/// everyone in the game (see `WsSession`'s `StreamHandler` for +/// `ws::Message::Text`). Because the sender is itself a member of the game's +/// broadcast set, it receives its own move back, stamped with the server +/// `version` field. (This previously asserted a no-op gap; the Text handler +/// now broadcasts, so the test asserts that behavior instead.) #[actix_web::test] -async fn client_sent_move_message_currently_produces_no_response() { +async fn client_sent_move_is_parsed_and_broadcast_back() { let (srv, _lobby) = start_ws_test_server(); - let game_id = "integration-game-noop".to_string(); - let url = format!("{}/v1/ws/game/{}", srv.url(""), game_id).replace("http://", "ws://"); + let game_id = "integration-game-echo".to_string(); + let url = srv.url(&format!("/v1/ws/game/{}", game_id)).replace("http://", "ws://"); let token = make_access_token(9, "client_move_sender"); let (_resp, mut connection) = awc::Client::new() @@ -224,17 +218,46 @@ async fn client_sent_move_message_currently_produces_no_response() { .await .unwrap(); + let fen = "rnbqkbnr/pppppppp/8/8/4P3/8/PPPP1PPP/RNBQKBNR b KQkq - 0 1"; let client_move = serde_json::json!({ "type": "Move", - "payload": { "from": "e2", "to": "e4", "san": "e4", "fen": "..." } + "payload": { "from": "e2", "to": "e4", "san": "e4", "fen": fen } }); connection.send(awc::ws::Message::Text(client_move.to_string().into())).await.unwrap(); - // Race the (lack of a) response against a short timeout — if this ever - // starts failing because a response *does* arrive, that's good news: - // it means the no-op gap above has been fixed, and this test (along - // with its doc comment) should be updated to assert the new behavior - // instead of removed outright. - let outcome = tokio::time::timeout(std::time::Duration::from_millis(300), connection.next()).await; - assert!(outcome.is_err(), "expected no response to a client-sent Move (current behavior is a no-op)"); + // The move is parsed and broadcast back to the sender within a short window. + let item = tokio::time::timeout(std::time::Duration::from_secs(2), connection.next()) + .await + .expect("expected the client's move to be broadcast back within the timeout") + .expect("connection closed before receiving the broadcast") + .expect("WS transport error"); + + let text = match item { + awc::ws::Frame::Text(bytes) => String::from_utf8(bytes.to_vec()).unwrap(), + other => panic!("expected a text frame, got {other:?}"), + }; + + let value: serde_json::Value = serde_json::from_str(&text).unwrap(); + assert_eq!(value["version"], "1.0", "server-sent messages are version-stamped"); + + let received: WsMessage = serde_json::from_value({ + let mut v = value.clone(); + if let serde_json::Value::Object(ref mut m) = v { + m.remove("version"); + } + v + }) + .expect("broadcast should round-trip back into WsMessage"); + + assert_eq!( + received, + WsMessage::Move { + from: "e2".into(), + to: "e4".into(), + san: "e4".into(), + fen: fen.into(), + } + ); + + let _ = connection.close().await; } \ No newline at end of file diff --git a/backend/modules/service/src/games.rs b/backend/modules/service/src/games.rs index b60e5a3..2ce3e49 100644 --- a/backend/modules/service/src/games.rs +++ b/backend/modules/service/src/games.rs @@ -614,8 +614,10 @@ mod tests { // We need two query result sets: one for count, one for the main query let db = MockDatabase::new(DbBackend::Postgres) .append_query_results(vec![ - // First query result (count) - vec![], + // First query result (count) — empty set; typed so `T: IntoMockRow` + // can be inferred. count() on no rows resolves to 0 and execution + // continues to the data query below. + Vec::::new(), ]) .append_query_results(vec![ // Second query result (main data) @@ -654,7 +656,9 @@ mod tests { // We expect two queries (count + data) assert_eq!(transaction_log.len(), 2); - let log = &transaction_log[0]; + // Inspect the data query (index 1); index 0 is the COUNT query, which + // carries neither the ORDER BY / LIMIT nor the keyset cursor predicate. + let log = &transaction_log[1]; let log_str = format!("{:?}", log); println!("Log: {}", log_str); @@ -676,8 +680,10 @@ mod tests { let db = MockDatabase::new(DbBackend::Postgres) .append_query_results(vec![ - // First query result (count) - vec![], + // First query result (count) — empty set; typed so `T: IntoMockRow` + // can be inferred. count() on no rows resolves to 0 and execution + // continues to the data query below. + Vec::::new(), ]) .append_query_results(vec![ // Second query result (main data) @@ -708,7 +714,9 @@ mod tests { ).await; let transaction_log = db.into_transaction_log(); - let log = &transaction_log[0]; + // Inspect the data query (index 1); index 0 is the COUNT query, which + // carries neither the ORDER BY / LIMIT nor the keyset cursor predicate. + let log = &transaction_log[1]; let log_str = format!("{:?}", log); println!("Log with cursor: {}", log_str);