Add --rp-purge/-X option to remove previous test results before execution#348
Merged
Conversation
Reviewer's GuideImplements a new --rp-purge/-X execute option that, when enabled, loads prior execute job metadata to identify per-request ReportPortal test suites via a newa_batch tag and removes them through new ReportPortal service delete/search helpers before re-submitting Testing Farm requests, plus updates CLI context wiring and README docs. Sequence diagram for execute with --rp-purge purge flowsequenceDiagram
actor User
participant CLI as cmd_execute
participant Ctx as CLIContext
participant Worker as tf_worker
participant RPInit as initialize_rp_connection
participant RP as ReportPortalService
participant TF as TestingFarm
User->>CLI: newa execute --rp-purge ...
CLI->>Ctx: set rp_purge = True
CLI->>Worker: start tf_worker(ctx, schedule_job)
Worker->>Worker: compute execute_job_file path
Worker->>Worker: ExecuteJob.from_yaml_file(...)
Worker->>Worker: decide start_new_request
alt start_new_request and ctx.rp_purge and reportportal configured
Worker->>Worker: read launch_uuid from schedule_job.request.reportportal
Worker->>Worker: read newa_batch_id from execute_job.execution.batch_id
Worker->>RPInit: initialize_rp_connection(ctx)
RPInit-->>Worker: ReportPortalService instance
Worker->>RP: remove_test_suite_by_tag(launch_uuid, newa_batch_id, logger)
RP->>RP: get_launch_info(launch_uuid)
RP-->>RP: launch_id
RP->>RP: get_request(/item, filter by launchId, type=suite, newa_batch)
RP-->>RP: items content
loop for each matching suite
RP->>RP: delete_request(/item/{item_id})
end
RP-->>Worker: removed = True/False
end
Worker->>Request: initiate_tf_request(ctx)
Request->>TF: create TF request
TF-->>Worker: TFRequest(uuid)
Worker-->>Ctx: save_execute_job(execute_job)
Class diagram for new ReportPortal purge supportclassDiagram
class CLIContext {
bool continue_execution
bool no_wait
bool rp_purge
list~str~ restart_request
list~RequestResult~ restart_result
}
class ReportPortalService {
url
project
token
post_request(path: str, data: dict, version: int) dict
get_request(path: str, params: dict, version: int) dict
get_launch_info(launch_uuid: str) dict
delete_request(path: str, version: int) bool
remove_test_suite_by_tag(launch_uuid: str, newa_batch_id: str, logger: logging.Logger) bool
}
class ExecuteJob {
+from_yaml_file(path: Path) ExecuteJob
execution: Execution
request: Request
}
class Execution {
result: RequestResult
request_api
request_uuid: str
batch_id: str
}
class ScheduleJob {
request: Request
}
class Request {
id: str
reportportal: dict
initiate_tf_request(ctx: CLIContext) TFRequest
}
class TFRequest {
api
uuid: str
}
CLIContext "1" --> "many" ScheduleJob : used_by
ScheduleJob "1" --> "1" Request : contains
ExecuteJob "1" --> "1" Execution : has
ExecuteJob "1" --> "1" Request : has
Execution "1" --> "1" RequestResult : uses
Request "1" --> "1" TFRequest : creates
CLIContext "1" --> "1" ReportPortalService : initializes
ReportPortalService "1" --> "many" TFRequest : reports_results_for
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
remove_test_suite_by_tag, the/itemlookup appears to assume a single page of results; if ReportPortal paginates this endpoint you may need to handle pagination to ensure all suites with the givennewa_batchtag are removed. - The current implementation of
remove_test_suite_by_tagaborts and returnsFalseon the first failed delete; consider deleting all matching items and aggregating/logging failures so that one problematic suite does not prevent others from being purged.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `remove_test_suite_by_tag`, the `/item` lookup appears to assume a single page of results; if ReportPortal paginates this endpoint you may need to handle pagination to ensure all suites with the given `newa_batch` tag are removed.
- The current implementation of `remove_test_suite_by_tag` aborts and returns `False` on the first failed delete; consider deleting all matching items and aggregating/logging failures so that one problematic suite does not prevent others from being purged.
## Individual Comments
### Comment 1
<location path="newa/services/reportportal_service.py" line_range="218-227" />
<code_context>
+ 'filter.has.compositeAttribute': f'newa_batch:{newa_batch_id}',
+ }
+
+ # Get test items matching the criteria
+ items = self.get_request('/item', params=params, version=1)
+
+ if not items or not items.get('content'):
+ if logger:
+ logger.debug(
+ f'No test suite found with tag newa_batch={newa_batch_id} '
+ f'in launch {launch_uuid}')
+ return False
+
+ # Delete each matching suite
+ for item in items['content']:
+ item_id = item['id']
+ if logger:
</code_context>
<issue_to_address>
**issue:** ReportPortal item retrieval appears to handle only a single page of results.
This request to `/item` appears to process only the first page of `items['content']`. If the ReportPortal API paginates this endpoint, additional suites with the `newa_batch` tag will be skipped. Please confirm whether the response includes pagination metadata (e.g., `page`, `size`, `totalPages`) and, if so, iterate through all pages when `rp_purge` is used.
</issue_to_address>
### Comment 2
<location path="newa/cli/workers.py" line_range="45-54" />
<code_context>
- if execute_job_file.exists():
- execute_job = ExecuteJob.from_yaml_file(execute_job_file)
+ execute_job = None
+ # Try to load former execute_job in case --continue or --rp-purge will need it
+ parent = schedule_file.parent
+ name = schedule_file.name
+ from newa import EXECUTE_FILE_PREFIX, SCHEDULE_FILE_PREFIX
+ execute_job_file = Path(
+ os.path.join(
+ parent,
+ name.replace(
+ SCHEDULE_FILE_PREFIX,
+ EXECUTE_FILE_PREFIX,
+ 1)))
+ if execute_job_file.exists():
+ execute_job = ExecuteJob.from_yaml_file(execute_job_file)
+ if ctx.continue_execution:
</code_context>
<issue_to_address>
**suggestion (performance):** Eagerly loading execute_job for every run may be unnecessary overhead.
This now loads `execute_job` whenever the file exists, even when neither `--continue` nor `--rp-purge` is used. For many or large jobs, that extra I/O and deserialization could be costly. Consider only loading it when `ctx.continue_execution` or `ctx.rp_purge` is true.
Suggested implementation:
```python
execute_job = None
# Compute the path to a former execute_job; only load it if actually needed
parent = schedule_file.parent
name = schedule_file.name
from newa import EXECUTE_FILE_PREFIX, SCHEDULE_FILE_PREFIX
execute_job_file = Path(
os.path.join(
parent,
name.replace(
SCHEDULE_FILE_PREFIX,
EXECUTE_FILE_PREFIX,
1)))
if ctx.continue_execution or ctx.rp_purge:
if execute_job_file.exists():
execute_job = ExecuteJob.from_yaml_file(execute_job_file)
```
If there is logic immediately after the removed `if ctx.continue_execution:` (e.g. a block that assumes `execute_job` has been loaded), it may need to be re-indented or guarded to ensure it still only runs when `ctx.continue_execution` is true. Please adjust any such block to either:
- Nest it under `if ctx.continue_execution:` again (but without the load), or
- Use `if ctx.continue_execution and execute_job is not None:` depending on the intended behavior when the file is missing.
</issue_to_address>
### Comment 3
<location path="README.md" line_range="1544" />
<code_context>
#### Option `--restart-result`, `-r`
This option can be used to reschedule NEWA request that have ended with a particular result - `passed, failed, error`. For example, `--restart-result error`. Result can be either `passed`, `failed` or `error` where 'error' means that test execution hasn't been finished correctly. This option can be used multiple times. Implies `--continue`.
+#### Option `--rp-purge`, `-X`
</code_context>
<issue_to_address>
**issue (typo):** Use plural "requests" to match the verb "have" in this sentence.
Change "NEWA request that have" to "NEWA requests that have" for correct subject–verb agreement.
```suggestion
This option can be used to reschedule NEWA requests that have ended with a particular result - `passed, failed, error`. For example, `--restart-result error`. Result can be either `passed`, `failed` or `error` where 'error' means that test execution hasn't been finished correctly. This option can be used multiple times. Implies `--continue`.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
…tion This commit introduces a new --rp-purge/-X option for the execute command that removes previous test results from ReportPortal launches before executing new Testing Farm requests. This feature is useful when restarting specific TF requests to avoid duplicate test results in the same launch. Key changes: - Add --rp-purge/-X option to execute command with per-request removal - Implement delete_request() method in ReportPortal service - Add remove_test_suite_by_tag() to remove test suites by newa_batch ID - Update workers to load previous execute_job for both --continue and --force - Support suite removal with --restart-request, --restart-result, and --force - Fix ReportPortal API filtering using filter.has.compositeAttribute syntax - Update README with comprehensive documentation and examples Implementation details: - Test results are identified using newa_batch tag (unique per execution) - Only removes results for requests being executed, not entire launch - Works per-worker to ensure only scheduled requests are cleaned up - Launch UUID is converted to numeric ID for API compatibility - Loads previous execute_job regardless of --continue flag for batch_id access 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Enhance the NEWA agent definition to incorporate the new --rp-purge/-X option for ReportPortal result cleanup during test rescheduling. The agent will now: - Proactively ask users about removing previous test results when rescheduling tests with --restart-request, --restart-result, or --force - Explain that --rp-purge only removes results for requests being re-executed, not the entire launch - Only suggest this option during rescheduling operations, never for initial test runs - Provide correct command syntax with examples This makes the --rp-purge feature more discoverable and helps users maintain clean ReportPortal launches when restarting tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit introduces a new --rp-purge/-X option for the execute command that removes previous test results from ReportPortal launches before executing new Testing Farm requests. This feature is useful when restarting specific TF requests to avoid duplicate test results in the same launch.
Key changes:
Implementation details:
🤖 Generated with Claude Code
Summary by Sourcery
Add support for purging previous ReportPortal test results before executing Testing Farm requests and document the new behavior.
New Features:
Enhancements:
Documentation: