Skip to content

add test coverage for all extrnal cli commands/surface [AI] - #1722

Open
rfbgo wants to merge 1 commit into
GoogleCloudPlatform:developfrom
rfbgo:clitest
Open

add test coverage for all extrnal cli commands/surface [AI]#1722
rfbgo wants to merge 1 commit into
GoogleCloudPlatform:developfrom
rfbgo:clitest

Conversation

@rfbgo

@rfbgo rfbgo commented Aug 13, 2026

Copy link
Copy Markdown
Collaborator

This PR aims to make sure every top level command an external user could rely on has some test coverage

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request expands test coverage for several Ramble CLI commands (such as help, config, clean, workspace, results, and python) and resolves a duplicate pipeline execution bug in the workspace push-to-cache command. The review feedback points out that the deletion of the commands test file removes existing test coverage, which may be accidental. Additionally, it identifies a mismatch in the clean command tests where the '-a' and '--all' flags do not assert against the newly added 'reports' effect.

I am having trouble creating individual review comments. Click here to see my feedback.

lib/ramble/ramble/test/commands.py (1-53)

high

The file lib/ramble/ramble/test/commands.py has been completely removed in this pull request. This removes all test coverage for the commands CLI command, which seems to contradict the PR's objective of adding test coverage for all external CLI commands/surface. Please verify if this deletion was accidental or if these tests were moved elsewhere.

lib/ramble/ramble/test/cmd/clean.py (58-59)

medium

The expected effects for "-a" and "--all" are hardcoded to ["downloads", "caches", "python_caches"], which excludes "reports". However, all_effects has been updated to include "reports". If the clean -a / clean --all command is intended to clean everything (including reports), these test cases should use all_effects instead of the hardcoded list to avoid test failures or incomplete coverage.

        (["-a"], all_effects),
        (["--all"], all_effects),

@rfbgo
rfbgo force-pushed the clitest branch 2 times, most recently from 4b3ca28 to f9f8285 Compare August 13, 2026 15:21
@rfbgo rfbgo changed the title add test coverage for all extrnal cli commands/surface add test coverage for all extrnal cli commands/surface [AI] Aug 13, 2026
@ramble-pr-bot

ramble-pr-bot Bot commented Aug 13, 2026

Copy link
Copy Markdown

Ramble Performance Test Metrics

Results produced with commit: 5e5e168

Test Name Outcome Duration (s) Most Recent Run (s) Last 5 Avg (s)
test_analyze_large_file passed 2.1484 2.0757 (e717f5a) 2.0866
test_large_template_expansion passed 1.7396 1.5651 (e717f5a) 1.5998
test_many_experiments passed 26.1580 24.0042 (e717f5a) 24.6420
test_many_objects_defaults passed 14.6834 13.5530 (e717f5a) 14.4205
test_matrix_filter_perf passed 1.1499 1.6413 (e717f5a) 1.6722

@codecov

codecov Bot commented Aug 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.91%. Comparing base (e717f5a) to head (5e5e168).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1722      +/-   ##
===========================================
+ Coverage    93.63%   93.91%   +0.28%     
===========================================
  Files          367      367              
  Lines        36307    36468     +161     
===========================================
+ Hits         33996    34250     +254     
+ Misses        2311     2218      -93     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant