Skip to content

Commit 9bf5440

Browse files
committed
refactor: apply AAA pattern consistently to remaining SSH tests
- Refactor it_should_store_ssh_configuration_correctly() with clear AAA structure - Refactor it_should_timeout_when_connecting_to_unreachable_host_with_real_ssh_infrastructure() using helper functions - Refactor it_should_execute_remote_command_on_real_ssh_server() with proper separation of concerns - All 6 SSH integration tests now follow consistent AAA (Arrange-Act-Assert) pattern - Improve test readability and maintainability following project conventions - Complete Phase 2.1: Apply AAA Pattern Consistently
1 parent eeb348d commit 9bf5440

2 files changed

Lines changed: 29 additions & 78 deletions

File tree

docs/refactors/ssh-client-integration-tests-refactor.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,7 @@ Improve test organization and consistency following project conventions.
285285

286286
### Proposal #3: Apply AAA Pattern Consistently
287287

288-
**Status**: 📋 Not Started
288+
**Status**: ✅ COMPLETED
289289
**Impact**: 🟢🟢🟢 Medium
290290
**Effort**: 🔵🔵 Low
291291
**Priority**: P1 (Required by project testing conventions)

tests/ssh_client_integration.rs

Lines changed: 28 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -347,40 +347,32 @@ async fn it_should_handle_connectivity_timeouts() {
347347
/// 3. Demonstrates configuration validation patterns
348348
#[tokio::test]
349349
async fn it_should_store_ssh_configuration_correctly() {
350-
// Define test parameters
350+
// Arrange: Set up test parameters and SSH client
351351
let test_ip = IpAddr::V4(Ipv4Addr::new(10, 0, 0, 100));
352352
let test_port = 2222;
353353
let test_username = "testuser";
354+
let private_key_path = PathBuf::from("/path/to/private_key");
355+
let public_key_path = PathBuf::from("/path/to/public_key.pub");
354356

355-
// Create SSH credentials
356357
let ssh_credentials = SshCredentials::new(
357-
PathBuf::from("/path/to/private_key"),
358-
PathBuf::from("/path/to/public_key.pub"),
358+
private_key_path.clone(),
359+
public_key_path.clone(),
359360
Username::new(test_username).unwrap(),
360361
);
361-
362-
// Create SSH configuration
363362
let ssh_config = SshConfig::new(ssh_credentials, SocketAddr::new(test_ip, test_port));
364-
365-
// Create SSH client
366363
let ssh_client = SshClient::new(ssh_config);
367364

368-
// Verify configuration is stored correctly
365+
// Act: Configuration is stored during client creation (no explicit action needed)
366+
367+
// Assert: Verify all configuration values are stored correctly
369368
assert_eq!(ssh_client.ssh_config().host_ip(), test_ip);
370369
assert_eq!(ssh_client.ssh_config().ssh_port(), test_port);
371370
assert_eq!(ssh_client.ssh_config().ssh_username(), test_username);
372-
373-
// Verify key paths are stored correctly
374371
assert_eq!(
375372
ssh_client.ssh_config().ssh_priv_key_path(),
376-
&PathBuf::from("/path/to/private_key")
373+
&private_key_path
377374
);
378-
assert_eq!(
379-
ssh_client.ssh_config().ssh_pub_key_path(),
380-
&PathBuf::from("/path/to/public_key.pub")
381-
);
382-
383-
println!("SSH configuration validation completed successfully");
375+
assert_eq!(ssh_client.ssh_config().ssh_pub_key_path(), &public_key_path);
384376
}
385377

386378
// =============================================================================
@@ -435,37 +427,11 @@ async fn it_should_connect_to_real_ssh_server_and_test_connectivity() {
435427
/// timeouts work in a real Docker environment.
436428
#[tokio::test]
437429
async fn it_should_timeout_when_connecting_to_unreachable_host_with_real_ssh_infrastructure() {
438-
// Use an unreachable IP address (RFC 5737 TEST-NET-1)
439-
let unreachable_ip = IpAddr::V4("192.0.2.1".parse().unwrap());
440-
441-
let ssh_credentials = SshCredentials::new(
442-
PathBuf::from("/nonexistent/key"), // Not used for password auth
443-
PathBuf::from("/nonexistent/key.pub"), // Not used for password auth
444-
Username::new("testuser").unwrap(),
445-
);
446-
447-
let ssh_config = SshConfig::new(ssh_credentials, SocketAddr::new(unreachable_ip, 22));
448-
449-
let ssh_client = SshClient::new(ssh_config);
450-
451-
// Measure timeout duration
452-
let start = std::time::Instant::now();
453-
let result = ssh_client.test_connectivity();
454-
let duration = start.elapsed();
455-
456-
// Verify timeout behavior
457-
assert!(
458-
result.is_err() || !result.unwrap(),
459-
"Connection to unreachable host should fail"
460-
);
461-
462-
// Should timeout around 5 seconds (with some tolerance for system variations)
463-
assert!(
464-
duration >= Duration::from_secs(4) && duration <= Duration::from_secs(10),
465-
"Timeout should be around 5 seconds, was: {duration:?}"
466-
);
430+
// Arrange: Set up SSH client configured to connect to unreachable host
431+
let client = SshTestBuilder::new().with_unreachable_host().build_client();
467432

468-
println!("Real SSH infrastructure timeout test completed in {duration:?}");
433+
// Act & Assert: Test connectivity should fail quickly for unreachable host
434+
assert_connectivity_fails_quickly(&client, 10);
469435
}
470436

471437
/// Test remote command execution using a real Docker SSH server
@@ -492,7 +458,7 @@ async fn it_should_timeout_when_connecting_to_unreachable_host_with_real_ssh_inf
492458
/// The test will skip gracefully if Docker is not available or the image is not built.
493459
#[tokio::test]
494460
async fn it_should_execute_remote_command_on_real_ssh_server() {
495-
// Start real SSH server container
461+
// Arrange: Set up real SSH server container and client
496462
let ssh_container = match RealSshServerContainer::start().await {
497463
Ok(container) => container,
498464
Err(e) => {
@@ -502,24 +468,12 @@ async fn it_should_execute_remote_command_on_real_ssh_server() {
502468
}
503469
};
504470

505-
// Create SSH credentials using the test SSH keys
506-
// NOTE: These keys must match the hardcoded public key in the SSH server Dockerfile
507-
let ssh_credentials = SshCredentials::new(
508-
PathBuf::from("fixtures/testing_rsa"), // Test private key
509-
PathBuf::from("fixtures/testing_rsa.pub"), // Test public key (hardcoded in Docker image)
510-
Username::new(ssh_container.test_username()).unwrap(),
511-
);
512-
513-
// Create SSH configuration
514-
let ssh_config = SshConfig::new(
515-
ssh_credentials,
516-
SocketAddr::new(ssh_container.host_ip(), ssh_container.ssh_port()),
517-
);
518-
519-
let ssh_client = SshClient::new(ssh_config);
471+
let client = SshTestBuilder::new()
472+
.with_real_container(&ssh_container)
473+
.build_client();
520474

521-
// Wait for SSH connectivity to be established
522-
match ssh_client.wait_for_connectivity().await {
475+
// Ensure SSH connectivity before command execution
476+
match client.wait_for_connectivity().await {
523477
Ok(()) => {
524478
println!("SSH connectivity established successfully");
525479
}
@@ -529,32 +483,29 @@ async fn it_should_execute_remote_command_on_real_ssh_server() {
529483
}
530484
}
531485

532-
// Execute a simple echo command
486+
// Act: Execute commands via SSH
533487
let test_message = "Hello SSH Integration Test";
534-
let command = format!("echo '{test_message}'");
488+
let echo_command = format!("echo '{test_message}'");
489+
let echo_result = client.execute(&echo_command);
490+
let whoami_result = client.execute("whoami");
535491

536-
match ssh_client.execute(&command) {
492+
// Assert: Verify command execution results
493+
match echo_result {
537494
Ok(output) => {
538495
let trimmed_output = output.trim();
539-
println!("Command '{command}' executed successfully. Output: '{trimmed_output}'");
540-
541-
// Verify the output matches our expected message
542496
assert_eq!(
543497
trimmed_output, test_message,
544-
"Command output should match expected message"
498+
"Echo command output should match expected message"
545499
);
546500
}
547501
Err(e) => {
548-
panic!("SSH command execution should succeed with real server: {e}");
502+
panic!("Echo command execution should succeed with real server: {e}");
549503
}
550504
}
551505

552-
// Execute another command to verify multiple executions work
553-
let whoami_result = ssh_client.execute("whoami");
554506
match whoami_result {
555507
Ok(output) => {
556508
let username = output.trim();
557-
println!("whoami command output: '{username}'");
558509
assert_eq!(
559510
username,
560511
ssh_container.test_username(),

0 commit comments

Comments
 (0)