Skip to content

Commit 8ff34ac

Browse files
joshrotenbergjoshrotenberg
committed
fix(cli): standardize slow-log command help text and add comprehensive tests
- Fixed slow-log description inconsistency ('View' vs 'Get slow query log') - Added default values (100, 0) for limit/offset in fixed-database slow-log - Standardized offset description to 'Offset for pagination' - Changed limit/offset from Option<i32> to i32 with defaults for consistency Added comprehensive assert_cmd tests to ensure documentation accuracy: - 14 new tests for profile --type flag and backward compatibility - 8 new tests for slow-log command consistency - Updated existing test to check for --type instead of --deployment - All tests verify help text matches actual command behavior These tests will catch documentation drift and ensure users can rely on our help text and examples.
1 parent ff2f8d6 commit 8ff34ac

File tree

3 files changed

+359
-7
lines changed

3 files changed

+359
-7
lines changed

‎crates/redisctl/src/cli.rs‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -984,17 +984,17 @@ pub enum CloudFixedDatabaseCommands {
984984
#[command(flatten)]
985985
async_ops: crate::commands::cloud::async_utils::AsyncOperationArgs,
986986
},
987-
/// View slow query log
987+
/// Get slow query log
988988
#[command(name = "slow-log")]
989989
SlowLog {
990990
/// Database ID (format: subscription_id:database_id)
991991
id: String,
992992
/// Maximum number of entries to return
993-
#[arg(long)]
994-
limit: Option<i32>,
995-
/// Number of entries to skip
996-
#[arg(long)]
997-
offset: Option<i32>,
993+
#[arg(long, default_value = "100")]
994+
limit: i32,
995+
/// Offset for pagination
996+
#[arg(long, default_value = "0")]
997+
offset: i32,
998998
},
999999
/// List tags for fixed database
10001000
#[command(name = "list-tags")]

‎crates/redisctl/tests/cli_basic_tests.rs‎

Lines changed: 127 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ fn test_profile_set_missing_deployment_type() {
235235
.arg("key")
236236
.assert()
237237
.failure()
238-
.stderr(predicate::str::contains("--deployment"));
238+
.stderr(predicate::str::contains("--type"));
239239
}
240240

241241
#[test]
@@ -281,3 +281,129 @@ fn test_payment_method_list_help() {
281281
.success()
282282
.stdout(predicate::str::contains("List payment methods"));
283283
}
284+
285+
// === SLOW-LOG COMMAND TESTS ===
286+
287+
#[test]
288+
fn test_cloud_database_slow_log_help() {
289+
redisctl()
290+
.arg("cloud")
291+
.arg("database")
292+
.arg("slow-log")
293+
.arg("--help")
294+
.assert()
295+
.success()
296+
.stdout(predicate::str::contains("Get slow query log"))
297+
.stdout(predicate::str::contains("--limit"))
298+
.stdout(predicate::str::contains("--offset"));
299+
}
300+
301+
#[test]
302+
fn test_cloud_database_slow_log_has_default_limit() {
303+
redisctl()
304+
.arg("cloud")
305+
.arg("database")
306+
.arg("slow-log")
307+
.arg("--help")
308+
.assert()
309+
.success()
310+
.stdout(predicate::str::contains("default: 100"));
311+
}
312+
313+
#[test]
314+
fn test_cloud_database_slow_log_has_default_offset() {
315+
redisctl()
316+
.arg("cloud")
317+
.arg("database")
318+
.arg("slow-log")
319+
.arg("--help")
320+
.assert()
321+
.success()
322+
.stdout(predicate::str::contains("default: 0"));
323+
}
324+
325+
#[test]
326+
fn test_cloud_fixed_database_slow_log_help() {
327+
redisctl()
328+
.arg("cloud")
329+
.arg("fixed-database")
330+
.arg("slow-log")
331+
.arg("--help")
332+
.assert()
333+
.success()
334+
.stdout(predicate::str::contains("Get slow query log"))
335+
.stdout(predicate::str::contains("--limit"))
336+
.stdout(predicate::str::contains("--offset"));
337+
}
338+
339+
#[test]
340+
fn test_cloud_fixed_database_slow_log_has_defaults() {
341+
redisctl()
342+
.arg("cloud")
343+
.arg("fixed-database")
344+
.arg("slow-log")
345+
.arg("--help")
346+
.assert()
347+
.success()
348+
.stdout(predicate::str::contains("default: 100"))
349+
.stdout(predicate::str::contains("default: 0"));
350+
}
351+
352+
#[test]
353+
fn test_cloud_database_slow_log_offset_description() {
354+
// Both should use "Offset for pagination" consistently
355+
redisctl()
356+
.arg("cloud")
357+
.arg("database")
358+
.arg("slow-log")
359+
.arg("--help")
360+
.assert()
361+
.success()
362+
.stdout(predicate::str::contains("Offset for pagination"));
363+
}
364+
365+
#[test]
366+
fn test_cloud_fixed_database_slow_log_offset_description() {
367+
// Both should use "Offset for pagination" consistently
368+
redisctl()
369+
.arg("cloud")
370+
.arg("fixed-database")
371+
.arg("slow-log")
372+
.arg("--help")
373+
.assert()
374+
.success()
375+
.stdout(predicate::str::contains("Offset for pagination"));
376+
}
377+
378+
#[test]
379+
fn test_slow_log_descriptions_match() {
380+
// Ensure both commands have the same description
381+
let database_output = redisctl()
382+
.arg("cloud")
383+
.arg("database")
384+
.arg("slow-log")
385+
.arg("--help")
386+
.assert()
387+
.success()
388+
.get_output()
389+
.stdout
390+
.clone();
391+
392+
let fixed_database_output = redisctl()
393+
.arg("cloud")
394+
.arg("fixed-database")
395+
.arg("slow-log")
396+
.arg("--help")
397+
.assert()
398+
.success()
399+
.get_output()
400+
.stdout
401+
.clone();
402+
403+
let database_desc = String::from_utf8_lossy(&database_output);
404+
let fixed_database_desc = String::from_utf8_lossy(&fixed_database_output);
405+
406+
// Both should say "Get slow query log"
407+
assert!(database_desc.contains("Get slow query log"));
408+
assert!(fixed_database_desc.contains("Get slow query log"));
409+
}

‎crates/redisctl/tests/cli_profile_tests.rs‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ fn test_cmd(temp_dir: &TempDir) -> Command {
1010
cmd
1111
}
1212

13+
/// Helper to get redisctl command without temp dir
14+
fn redisctl() -> Command {
15+
Command::cargo_bin("redisctl").unwrap()
16+
}
17+
1318
#[test]
1419
fn test_profile_list() {
1520
let temp_dir = TempDir::new().unwrap();
@@ -413,3 +418,224 @@ fn test_profile_set_cloud_with_custom_url() {
413418
.assert()
414419
.success();
415420
}
421+
422+
// === NEW TESTS FOR --type FLAG AND HELP TEXT ACCURACY ===
423+
424+
#[test]
425+
fn test_profile_set_with_type_flag_cloud() {
426+
let temp_dir = TempDir::new().unwrap();
427+
428+
// Test new --type flag
429+
test_cmd(&temp_dir)
430+
.arg("profile")
431+
.arg("set")
432+
.arg("type-test-cloud")
433+
.arg("--type")
434+
.arg("cloud")
435+
.arg("--api-key")
436+
.arg("test-key")
437+
.arg("--api-secret")
438+
.arg("test-secret")
439+
.assert()
440+
.success();
441+
442+
// Verify profile was created
443+
test_cmd(&temp_dir)
444+
.arg("profile")
445+
.arg("list")
446+
.assert()
447+
.success()
448+
.stdout(predicate::str::contains("type-test-cloud"));
449+
}
450+
451+
#[test]
452+
fn test_profile_set_with_type_flag_enterprise() {
453+
let temp_dir = TempDir::new().unwrap();
454+
455+
// Test new --type flag
456+
test_cmd(&temp_dir)
457+
.arg("profile")
458+
.arg("set")
459+
.arg("type-test-enterprise")
460+
.arg("--type")
461+
.arg("enterprise")
462+
.arg("--url")
463+
.arg("https://localhost:9443")
464+
.arg("--username")
465+
.arg("[email protected]")
466+
.arg("--password")
467+
.arg("password123")
468+
.assert()
469+
.success();
470+
471+
// Verify profile was created
472+
test_cmd(&temp_dir)
473+
.arg("profile")
474+
.arg("list")
475+
.assert()
476+
.success()
477+
.stdout(predicate::str::contains("type-test-enterprise"));
478+
}
479+
480+
#[test]
481+
fn test_profile_set_deployment_alias_still_works() {
482+
let temp_dir = TempDir::new().unwrap();
483+
484+
// Verify --deployment alias still works for backward compatibility
485+
test_cmd(&temp_dir)
486+
.arg("profile")
487+
.arg("set")
488+
.arg("alias-test")
489+
.arg("--deployment")
490+
.arg("cloud")
491+
.arg("--api-key")
492+
.arg("key")
493+
.arg("--api-secret")
494+
.arg("secret")
495+
.assert()
496+
.success();
497+
}
498+
499+
#[test]
500+
fn test_profile_set_help_shows_type_flag() {
501+
// Verify help shows --type as the primary flag
502+
redisctl()
503+
.arg("profile")
504+
.arg("set")
505+
.arg("--help")
506+
.assert()
507+
.success()
508+
.stdout(predicate::str::contains("--type <TYPE>"))
509+
.stdout(predicate::str::contains("Platform type"));
510+
}
511+
512+
#[test]
513+
fn test_profile_set_help_shows_deployment_alias() {
514+
// Verify help shows --deployment as an alias
515+
redisctl()
516+
.arg("profile")
517+
.arg("set")
518+
.arg("--help")
519+
.assert()
520+
.success()
521+
.stdout(predicate::str::contains("--deployment"));
522+
}
523+
524+
#[test]
525+
fn test_profile_set_help_examples_use_type_flag() {
526+
// Verify examples use --type, not bare positional argument
527+
redisctl()
528+
.arg("profile")
529+
.arg("set")
530+
.arg("--help")
531+
.assert()
532+
.success()
533+
.stdout(predicate::str::contains("--type cloud"))
534+
.stdout(predicate::str::contains("--type enterprise"));
535+
}
536+
537+
#[test]
538+
fn test_profile_help_examples_accurate() {
539+
// Verify main profile help has accurate examples
540+
redisctl()
541+
.arg("profile")
542+
.arg("--help")
543+
.assert()
544+
.success()
545+
.stdout(predicate::str::contains("profile set mycloud --type cloud"))
546+
.stdout(predicate::str::contains(
547+
"profile set myenterprise --type enterprise",
548+
));
549+
}
550+
551+
#[test]
552+
fn test_main_help_profile_examples_accurate() {
553+
// Verify main redisctl help has accurate profile examples
554+
redisctl()
555+
.arg("--help")
556+
.assert()
557+
.success()
558+
.stdout(predicate::str::contains("profile set mycloud --type cloud"))
559+
.stdout(predicate::str::contains(
560+
"profile set myenterprise --type enterprise",
561+
));
562+
}
563+
564+
#[test]
565+
fn test_profile_set_requires_type_flag() {
566+
let temp_dir = TempDir::new().unwrap();
567+
568+
// Should fail without --type flag
569+
test_cmd(&temp_dir)
570+
.arg("profile")
571+
.arg("set")
572+
.arg("test-profile")
573+
.arg("--api-key")
574+
.arg("key")
575+
.arg("--api-secret")
576+
.arg("secret")
577+
.assert()
578+
.failure()
579+
.stderr(predicate::str::contains("--type"));
580+
}
581+
582+
#[test]
583+
fn test_profile_list_help() {
584+
redisctl()
585+
.arg("profile")
586+
.arg("list")
587+
.arg("--help")
588+
.assert()
589+
.success()
590+
.stdout(predicate::str::contains("List all configured profiles"));
591+
}
592+
593+
#[test]
594+
fn test_profile_show_help() {
595+
redisctl()
596+
.arg("profile")
597+
.arg("show")
598+
.arg("--help")
599+
.assert()
600+
.success()
601+
.stdout(predicate::str::contains(
602+
"Show details of a specific profile",
603+
));
604+
}
605+
606+
#[test]
607+
fn test_profile_validate_help() {
608+
redisctl()
609+
.arg("profile")
610+
.arg("validate")
611+
.arg("--help")
612+
.assert()
613+
.success()
614+
.stdout(predicate::str::contains(
615+
"Validate configuration file and profiles",
616+
));
617+
}
618+
619+
#[test]
620+
fn test_profile_remove_help() {
621+
redisctl()
622+
.arg("profile")
623+
.arg("remove")
624+
.arg("--help")
625+
.assert()
626+
.success()
627+
.stdout(predicate::str::contains("Remove a profile"));
628+
}
629+
630+
#[test]
631+
fn test_profile_path_help() {
632+
redisctl()
633+
.arg("profile")
634+
.arg("path")
635+
.arg("--help")
636+
.assert()
637+
.success()
638+
.stdout(predicate::str::contains(
639+
"Show the path to the configuration file",
640+
));
641+
}

0 commit comments

Comments
 (0)