mirror of
https://github.com/ceph/ceph
synced 2026-08-02 07:03:18 +00:00
ptl-tool: add --log option to also write output to a per-run log file
Adds a --log flag to ptl-tool.py. When set (and not combined with --dry-run, which suppresses logging entirely), a per-run log file captures the whole run's output in the current working directory. The log filename uses this run's fully-resolved branch name (timestamp and base/release suffixes already applied, computed in build_branch()), falling back to the generic ptl-tool.log when no branch name is available. Records sent through the module logger land in the file via add_file_log_handler(), which also returns the resolved path. Plain print() output and interactive prompts/responses are captured through an explicit logged_input() helper that all input() call sites now use, rather than monkeypatching sys.stdout/builtins.input. Addresses review feedback from batrick: log path resolves against os.getcwd() instead of the git checkout directory, and prompt/response logging goes through an explicit helper instead of stdout/input monkeypatching. Signed-off-by: Yuri Weinstein <yweinste@redhat.com>
This commit is contained in:
Yuri Weinstein
parent
ff681c5884
commit
a9e14a1fa2
@ -153,6 +153,35 @@ log = logging.getLogger(__name__)
|
||||
log.addHandler(logging.StreamHandler())
|
||||
log.setLevel(logging.INFO)
|
||||
|
||||
|
||||
def add_file_log_handler(logger, label=None):
|
||||
"""--log support: also write logger's output to <label>.log (or ptl-tool.log
|
||||
when no label is given) in the current working directory, copying the
|
||||
console handler's formatter."""
|
||||
filename = f"{label}.log" if label else "ptl-tool.log"
|
||||
filepath = os.path.join(os.getcwd(), filename)
|
||||
fh = logging.FileHandler(filepath)
|
||||
fh.setLevel(logger.level)
|
||||
for handler in logger.handlers:
|
||||
if handler.formatter:
|
||||
fh.setFormatter(handler.formatter)
|
||||
break
|
||||
logger.addHandler(fh)
|
||||
return os.path.abspath(filepath)
|
||||
|
||||
|
||||
def logged_input(prompt=''):
|
||||
response = input(prompt)
|
||||
for handler in log.handlers:
|
||||
if isinstance(handler, logging.FileHandler):
|
||||
handler.acquire()
|
||||
try:
|
||||
handler.stream.write(f"{prompt}{response}\n")
|
||||
handler.flush()
|
||||
finally:
|
||||
handler.release()
|
||||
return response
|
||||
|
||||
# find containing git dir
|
||||
try:
|
||||
git_dir = git.Repo(GITDIR, search_parent_directories=True).working_tree_dir
|
||||
@ -516,7 +545,7 @@ def get_credits(session, pr, pr_req):
|
||||
credits.add("Reviewed-by: "+NEW_CONTRIBUTORS[user])
|
||||
except KeyError as e:
|
||||
try:
|
||||
name = input("Need name for contributor \"%s\" (use ^D to skip); Reviewed-by: " % user)
|
||||
name = logged_input("Need name for contributor \"%s\" (use ^D to skip); Reviewed-by: " % user)
|
||||
name = name.strip()
|
||||
if len(name) == 0:
|
||||
continue
|
||||
@ -673,13 +702,13 @@ def post_draft_review(session, pr, initial_text, base=None):
|
||||
with open(tf_path, 'r', encoding='utf-8') as f_read:
|
||||
final_text = f_read.read().strip()
|
||||
|
||||
print("\n" + "="*80)
|
||||
print("DRAFT REVIEW PREVIEW:")
|
||||
log.info("\n" + "="*80)
|
||||
log.info("DRAFT REVIEW PREVIEW:")
|
||||
print("-" * 80)
|
||||
print(final_text)
|
||||
print("="*80 + "\n")
|
||||
log.info(final_text)
|
||||
log.info("="*80 + "\n")
|
||||
|
||||
confirm = input(f"Post this feedback to PR #{pr}? [r/c/e/m/N] (r=request changes, c=comment, e=edit again, m=skip to merge, n=cancel): ").strip().lower()
|
||||
confirm = logged_input(f"Post this feedback to PR #{pr}? [r/c/e/m/N] (r=request changes, c=comment, e=edit again, m=skip to merge, n=cancel): ").strip().lower()
|
||||
if confirm == 'm':
|
||||
raise SkipToMerge()
|
||||
elif confirm in ('r', 'c'):
|
||||
@ -695,7 +724,7 @@ def post_draft_review(session, pr, initial_text, base=None):
|
||||
elif confirm == 'e':
|
||||
continue
|
||||
else:
|
||||
print("Review cancelled.")
|
||||
log.info("Review cancelled.")
|
||||
return False
|
||||
finally:
|
||||
os.unlink(tf_path)
|
||||
@ -793,11 +822,11 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
if found_prs:
|
||||
log.info(f"Original PRs identified in this backport: {', '.join(sorted(list(found_prs)))}")
|
||||
if len(found_prs) > 1:
|
||||
print("\033[91m" + "="*80)
|
||||
print("WARNING: Multiple original PRs detected in this backport!")
|
||||
print("Normally we expect exactly one main PR per backport.")
|
||||
print("Detected: " + ", ".join(sorted(list(found_prs))))
|
||||
print("="*80 + "\033[0m")
|
||||
log.info("\033[91m" + "="*80)
|
||||
log.info("WARNING: Multiple original PRs detected in this backport!")
|
||||
log.info("Normally we expect exactly one main PR per backport.")
|
||||
log.info("Detected: " + ", ".join(sorted(list(found_prs))))
|
||||
log.info("="*80 + "\033[0m")
|
||||
|
||||
return {
|
||||
'found_prs': found_prs,
|
||||
@ -901,11 +930,11 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
md_text += "\n[Be familiar with the rules and guidelines for writing backports.](https://github.com/ceph/ceph/blob/main/SubmittingPatches-backports.rst)\n\n"
|
||||
|
||||
while True:
|
||||
ans = input("Do you want to allow these commits anyway? [p/m/r/o/q] (p=proceed, m=skip to merge, r=add to review, o=open PR in browser, q=quit) ").strip().lower()
|
||||
ans = logged_input("Do you want to allow these commits anyway? [p/m/r/o/q] (p=proceed, m=skip to merge, r=add to review, o=open PR in browser, q=quit) ").strip().lower()
|
||||
if ans == 'o':
|
||||
url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{ctx.pr}"
|
||||
open_in_browser([url])
|
||||
print(f"Opened {url} in browser.")
|
||||
log.info(f"Opened {url} in browser.")
|
||||
elif ans == 'm':
|
||||
raise SkipToMerge()
|
||||
elif ans == 'r':
|
||||
@ -917,7 +946,7 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
elif ans == 'p':
|
||||
break
|
||||
else:
|
||||
print("Invalid choice. Please enter p, m, r, o, or q.")
|
||||
log.info("Invalid choice. Please enter p, m, r, o, or q.")
|
||||
|
||||
def _handle_unmerged_commits(self, ctx, unmerged_cps, cp_regex):
|
||||
md_text = "### Automated Backport Parity Review - Unmerged Commits Detected\n\n"
|
||||
@ -933,7 +962,7 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
ctx.report.add("Unmerged Cherry-Picks", md_text)
|
||||
else:
|
||||
while True:
|
||||
ans = input("Unmerged cherry-picks detected! [p]roceed, [m] skip to merge, [o]pen browser to investigate, [r]eview PR (add to review), [q]uit: ").strip().lower()
|
||||
ans = logged_input("Unmerged cherry-picks detected! [p]roceed, [m] skip to merge, [o]pen browser to investigate, [r]eview PR (add to review), [q]uit: ").strip().lower()
|
||||
if ans == 'p':
|
||||
break
|
||||
elif ans == 'm':
|
||||
@ -953,9 +982,9 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
urls_to_open.append(f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/commit/{orig_sha}")
|
||||
urls_to_open.append(f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/commit/{c.hexsha}")
|
||||
open_in_browser(urls_to_open)
|
||||
print("Opened URLs in browser.")
|
||||
log.info("Opened URLs in browser.")
|
||||
else:
|
||||
print("Invalid choice. Please enter p, m, o, r, or q.")
|
||||
log.info("Invalid choice. Please enter p, m, o, r, or q.")
|
||||
|
||||
def _handle_parity_mismatch(self, ctx, missing_commits, extra_commits, found_prs):
|
||||
md_text = f"### Automated Backport Parity Review\n\n"
|
||||
@ -968,7 +997,7 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
ctx.report.add("Parity Mismatch", md_text)
|
||||
else:
|
||||
while True:
|
||||
ans = input("Parity mismatch! [p]roceed, [m] skip to merge, [o]pen browser to investigate, [r]eview PR (add to review), [q]uit: ").strip().lower()
|
||||
ans = logged_input("Parity mismatch! [p]roceed, [m] skip to merge, [o]pen browser to investigate, [r]eview PR (add to review), [q]uit: ").strip().lower()
|
||||
if ans == 'p':
|
||||
break
|
||||
elif ans == 'm':
|
||||
@ -992,9 +1021,9 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
for c in extra_commits:
|
||||
urls_to_open.append(f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/commit/{c.hexsha}")
|
||||
open_in_browser(urls_to_open)
|
||||
print("Opened URLs in browser.")
|
||||
log.info("Opened URLs in browser.")
|
||||
else:
|
||||
print("Invalid choice. Please enter p, m, o, r, or q.")
|
||||
log.info("Invalid choice. Please enter p, m, o, r, or q.")
|
||||
|
||||
def _handle_multiple_prs(self, ctx, found_prs):
|
||||
md_text = f"### Automated Backport Parity Review - Multiple PRs Detected\n\n"
|
||||
@ -1005,7 +1034,7 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
ctx.report.add("Multiple Source PRs", md_text)
|
||||
else:
|
||||
while True:
|
||||
ans = input("Multiple original PRs detected! Do you want to add a review requesting justification? [p/r/m/o] (p=proceed/ignore, r=add to review, m=skip to merge, o=open PRs in browser): ").strip().lower()
|
||||
ans = logged_input("Multiple original PRs detected! Do you want to add a review requesting justification? [p/r/m/o] (p=proceed/ignore, r=add to review, m=skip to merge, o=open PRs in browser): ").strip().lower()
|
||||
if ans == 'o':
|
||||
url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{ctx.pr}"
|
||||
urls_to_open = [url]
|
||||
@ -1014,7 +1043,7 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
if m_pr:
|
||||
urls_to_open.append(f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{m_pr.group(1)}")
|
||||
open_in_browser(urls_to_open)
|
||||
print("Opened URLs in browser.")
|
||||
log.info("Opened URLs in browser.")
|
||||
elif ans == 'r':
|
||||
ctx.report.add("Multiple Source PRs", md_text)
|
||||
ctx.report.record_failure()
|
||||
@ -1024,7 +1053,7 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
elif ans == 'p':
|
||||
break
|
||||
else:
|
||||
print("Invalid choice. Please enter p, r, m, or o.")
|
||||
log.info("Invalid choice. Please enter p, r, m, or o.")
|
||||
|
||||
def run(self, ctx: AuditContext) -> None:
|
||||
log.info("Verifying commit parity with original PR(s) locally...")
|
||||
@ -1036,7 +1065,7 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
vis_text, vis_md = self._generate_visualizer(ctx.pr_commits, ctx.pr, mapping['pr_mapping'], mapping['missing_commits'])
|
||||
|
||||
if vis_text:
|
||||
print(vis_text)
|
||||
log.info(vis_text)
|
||||
if vis_md:
|
||||
ctx.report.set_visualizer(vis_md)
|
||||
|
||||
@ -1083,13 +1112,13 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
if ctx.args.ci_mode:
|
||||
ctx.report.add("Scrambled Commits", scramble_msg)
|
||||
else:
|
||||
print("\033[91m" + "="*80)
|
||||
print("WARNING: Scrambled commits detected in the backport!")
|
||||
log.info("\033[91m" + "="*80)
|
||||
log.info("WARNING: Scrambled commits detected in the backport!")
|
||||
for reason in scramble_reasons:
|
||||
print(f" - {reason}")
|
||||
print("="*80 + "\033[0m")
|
||||
log.info(f" - {reason}")
|
||||
log.info("="*80 + "\033[0m")
|
||||
while True:
|
||||
ans = input("Do you want to request changes for scrambled commits? [p/r/m/q] (p=proceed anyway, r=add to review, m=skip to merge, q=quit): ").strip().lower()
|
||||
ans = logged_input("Do you want to request changes for scrambled commits? [p/r/m/q] (p=proceed anyway, r=add to review, m=skip to merge, q=quit): ").strip().lower()
|
||||
if ans == 'm':
|
||||
raise SkipToMerge()
|
||||
elif ans == 'r':
|
||||
@ -1101,7 +1130,7 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
elif ans == 'p':
|
||||
break
|
||||
else:
|
||||
print("Invalid choice. Please enter p, r, m, or q.")
|
||||
log.info("Invalid choice. Please enter p, r, m, or q.")
|
||||
|
||||
if invalid_format_commits:
|
||||
self._handle_invalid_formats(ctx, invalid_format_commits)
|
||||
@ -1115,7 +1144,7 @@ class CommitParityCheck(BaseAuditCheck):
|
||||
if mapping['missing_commits'] or extra_commits:
|
||||
self._handle_parity_mismatch(ctx, mapping['missing_commits'], extra_commits, mapping['found_prs'])
|
||||
elif mapping['analyzed_merges']:
|
||||
print("\033[92mCommit parity check passed! All upstream commits from identified PRs are present.\033[0m")
|
||||
log.info("\033[92mCommit parity check passed! All upstream commits from identified PRs are present.\033[0m")
|
||||
if len(mapping['found_prs']) > 1:
|
||||
self._handle_multiple_prs(ctx, mapping['found_prs'])
|
||||
|
||||
@ -1192,11 +1221,11 @@ class ConflictSimulationCheck(BaseAuditCheck):
|
||||
raise SkipToMerge()
|
||||
|
||||
while True:
|
||||
ans = input("How do you want to handle this? [p/m/r/o/q] (p=proceed simulation anyway, m=skip to merge, r=add to review and skip simulation, o=open PR in browser, q=quit) ").strip().lower()
|
||||
ans = logged_input("How do you want to handle this? [p/m/r/o/q] (p=proceed simulation anyway, m=skip to merge, r=add to review and skip simulation, o=open PR in browser, q=quit) ").strip().lower()
|
||||
if ans == 'o':
|
||||
url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{pr}"
|
||||
open_in_browser([url])
|
||||
print(f"Opened {url} in browser.")
|
||||
log.info(f"Opened {url} in browser.")
|
||||
elif ans == 'm':
|
||||
raise SkipToMerge()
|
||||
elif ans == 'r':
|
||||
@ -1209,7 +1238,7 @@ class ConflictSimulationCheck(BaseAuditCheck):
|
||||
elif ans == 'p':
|
||||
break
|
||||
else:
|
||||
print("Invalid choice. Please enter p, m, r, o, or q.")
|
||||
log.info("Invalid choice. Please enter p, m, r, o, or q.")
|
||||
|
||||
backport_tree = wt_repo.head.commit.tree.hexsha
|
||||
|
||||
@ -1225,16 +1254,16 @@ class ConflictSimulationCheck(BaseAuditCheck):
|
||||
ans = 'i'
|
||||
else:
|
||||
while True:
|
||||
ans = input(f"Conflict or unapproved deviation detected in {c.hexsha[:8]}. Do you want to interactively review this PR? [i/a/m/o]\n(i = interactively review, a = auto-approve remaining, m = skip to merge, o = open in browser) ").strip().lower()
|
||||
ans = logged_input(f"Conflict or unapproved deviation detected in {c.hexsha[:8]}. Do you want to interactively review this PR? [i/a/m/o]\n(i = interactively review, a = auto-approve remaining, m = skip to merge, o = open in browser) ").strip().lower()
|
||||
if ans == 'o':
|
||||
bp_pr_url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{pr}"
|
||||
commit_url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/commit/{c.hexsha}"
|
||||
open_in_browser([bp_pr_url, commit_url])
|
||||
print("Opened PR and commit URLs in browser.")
|
||||
log.info("Opened PR and commit URLs in browser.")
|
||||
elif ans in ['i', 'a', 'm']:
|
||||
break
|
||||
else:
|
||||
print("Invalid choice. Please enter i, a, m, or o.")
|
||||
log.info("Invalid choice. Please enter i, a, m, or o.")
|
||||
first_conflict = False
|
||||
if ans == 'm':
|
||||
log.info("Skipping ahead to merge.")
|
||||
@ -1327,20 +1356,20 @@ class ConflictSimulationCheck(BaseAuditCheck):
|
||||
ans = 'r'
|
||||
break
|
||||
|
||||
ans = input(prompt_text).strip().lower()
|
||||
ans = logged_input(prompt_text).strip().lower()
|
||||
if ans == 'o':
|
||||
bp_pr_url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{pr}"
|
||||
orig_commit_url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/commit/{c.hexsha}"
|
||||
bp_commit_url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/commit/{commit.hexsha}"
|
||||
open_in_browser([bp_pr_url, orig_commit_url, bp_commit_url])
|
||||
print("Opened relevant URLs in browser.")
|
||||
log.info("Opened relevant URLs in browser.")
|
||||
elif ans == 'e':
|
||||
log.info(f"Re-opening {editor} to examine conflicts...")
|
||||
open_editor = True
|
||||
elif ans in ['p', 'r', 's', 'm']:
|
||||
break
|
||||
else:
|
||||
print("Invalid choice. Please enter p, r, s, m, o, or e.")
|
||||
log.info("Invalid choice. Please enter p, r, s, m, o, or e.")
|
||||
if ans == 'm':
|
||||
log.info("Skipping ahead to merge.")
|
||||
raise SkipToMerge()
|
||||
@ -1375,11 +1404,11 @@ class ConflictSimulationCheck(BaseAuditCheck):
|
||||
raise SkipToMerge()
|
||||
|
||||
while True:
|
||||
ans = input("How do you want to handle this? [p/m/r/o/q] (p=proceed simulation anyway, m=skip to merge, r=add to review and skip simulation, o=open PR in browser, q=quit) ").strip().lower()
|
||||
ans = logged_input("How do you want to handle this? [p/m/r/o/q] (p=proceed simulation anyway, m=skip to merge, r=add to review and skip simulation, o=open PR in browser, q=quit) ").strip().lower()
|
||||
if ans == 'o':
|
||||
url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{pr}"
|
||||
open_in_browser([url])
|
||||
print(f"Opened {url} in browser.")
|
||||
log.info(f"Opened {url} in browser.")
|
||||
elif ans == 'm':
|
||||
raise SkipToMerge()
|
||||
elif ans == 'r':
|
||||
@ -1392,7 +1421,7 @@ class ConflictSimulationCheck(BaseAuditCheck):
|
||||
elif ans == 'p':
|
||||
break
|
||||
else:
|
||||
print("Invalid choice. Please enter p, m, r, o, or q.")
|
||||
log.info("Invalid choice. Please enter p, m, r, o, or q.")
|
||||
|
||||
if recorded_deviations:
|
||||
md_text = """
|
||||
@ -1514,11 +1543,11 @@ class RedmineLinkageCheck(BaseAuditCheck):
|
||||
if match and int(match.group(1)) == orig_pr:
|
||||
main_trackers.append(issue)
|
||||
log.info(f"Found main tracker #{issue.id} ({REDMINE_ENDPOINT}/issues/{issue.id}) via description search.")
|
||||
print(f"Found PR #{orig_pr} in description of issue #{issue.id}.")
|
||||
log.info(f"Found PR #{orig_pr} in description of issue #{issue.id}.")
|
||||
if args.ci_mode:
|
||||
irregularities.append(f"**Malformed Main Tracker:** Main tracker [#{issue.id}]({REDMINE_ENDPOINT}/issues/{issue.id}) has the PR link in the description rather than the 'Pull Request ID' field. Please fix this.")
|
||||
else:
|
||||
ans = input(f"Fix tracker #{issue.id} by moving PR link from description to 'Pull Request ID' field? [y/N/m]: ").strip().lower()
|
||||
ans = logged_input(f"Fix tracker #{issue.id} by moving PR link from description to 'Pull Request ID' field? [y/N/m]: ").strip().lower()
|
||||
if ans == 'm':
|
||||
raise SkipToMerge()
|
||||
elif ans == 'y':
|
||||
@ -1569,11 +1598,11 @@ class RedmineLinkageCheck(BaseAuditCheck):
|
||||
if match:
|
||||
found_pr = match.group(1)
|
||||
log.debug(f"Found PR #{found_pr} in description of backport tracker #{bp_tracker.id}")
|
||||
print(f"Found PR #{found_pr} in description of backport tracker #{bp_tracker.id}.")
|
||||
log.info(f"Found PR #{found_pr} in description of backport tracker #{bp_tracker.id}.")
|
||||
if args.ci_mode:
|
||||
irregularities.append(f"**Malformed Backport Tracker:** Backport tracker [#{bp_tracker.id}]({REDMINE_ENDPOINT}/issues/{bp_tracker.id}) has the PR link in the description rather than the 'Pull Request ID' field. Please fix this.")
|
||||
else:
|
||||
ans = input(f"Fix tracker #{bp_tracker.id} by moving PR link from description to 'Pull Request ID' field? [y/N/m]: ").strip().lower()
|
||||
ans = logged_input(f"Fix tracker #{bp_tracker.id} by moving PR link from description to 'Pull Request ID' field? [y/N/m]: ").strip().lower()
|
||||
if ans == 'm':
|
||||
raise SkipToMerge()
|
||||
elif ans == 'y':
|
||||
@ -1610,15 +1639,15 @@ class RedmineLinkageCheck(BaseAuditCheck):
|
||||
if notes:
|
||||
md_text += "\n" + "\n".join([f"* {note}" for note in notes]) + "\n"
|
||||
|
||||
print("\n\033[93m" + "="*80)
|
||||
print("REDMINE LINKAGE IRREGULARITIES DETECTED")
|
||||
print("="*80 + "\033[0m")
|
||||
log.info("\n\033[93m" + "="*80)
|
||||
log.info("REDMINE LINKAGE IRREGULARITIES DETECTED")
|
||||
log.info("="*80 + "\033[0m")
|
||||
|
||||
if args.ci_mode:
|
||||
report.add("Redmine Linkage", md_text)
|
||||
else:
|
||||
while True:
|
||||
ans = input("Redmine irregularities detected! [p/r/m] (p=proceed, r=add to review, m=skip to merge): ").strip().lower()
|
||||
ans = logged_input("Redmine irregularities detected! [p/r/m] (p=proceed, r=add to review, m=skip to merge): ").strip().lower()
|
||||
if ans == 'p':
|
||||
break
|
||||
elif ans == 'm':
|
||||
@ -1662,11 +1691,11 @@ class MergeConflictCheck(BaseAuditCheck):
|
||||
report.add("Base Conflicts", md_text)
|
||||
else:
|
||||
while True:
|
||||
ans = input(f"PR #{pr} needs a rebase! Do you want to add a review requesting a rebase? [p/r/m/q/o] (p=proceed/skip check, r=add to review, m=skip to merge, q=abort, o=open PR in browser): ").strip().lower()
|
||||
ans = logged_input(f"PR #{pr} needs a rebase! Do you want to add a review requesting a rebase? [p/r/m/q/o] (p=proceed/skip check, r=add to review, m=skip to merge, q=abort, o=open PR in browser): ").strip().lower()
|
||||
if ans == 'o':
|
||||
url = f"https://github.com/{BASE_PROJECT}/{BASE_REPO}/pull/{pr}"
|
||||
open_in_browser([url])
|
||||
print(f"Opened {url} in browser.")
|
||||
log.info(f"Opened {url} in browser.")
|
||||
elif ans == 'm':
|
||||
raise SkipToMerge()
|
||||
elif ans == 'p':
|
||||
@ -1680,7 +1709,7 @@ class MergeConflictCheck(BaseAuditCheck):
|
||||
log.error(f"Aborting script due to unmergeable PR #{pr}.")
|
||||
sys.exit(1)
|
||||
else:
|
||||
print("Invalid choice. Please enter p, r, m, q, or o.")
|
||||
log.info("Invalid choice. Please enter p, r, m, q, or o.")
|
||||
|
||||
def write_ci_summary(pr: int, passed: bool, exit_code: int = 1):
|
||||
if not passed:
|
||||
@ -2063,10 +2092,10 @@ def build_branch(args):
|
||||
elif len(matching_tickets) == 1:
|
||||
t = matching_tickets[0]
|
||||
t_url = f"{REDMINE_ENDPOINT}/issues/{t.id}"
|
||||
print(f"\nFound existing open QA ticket: #{t.id} - {t.subject}")
|
||||
print(f"Link: {t_url}")
|
||||
log.info(f"\nFound existing open QA ticket: #{t.id} - {t.subject}")
|
||||
log.info(f"Link: {t_url}")
|
||||
while True:
|
||||
ans = input("Do you want to update this existing QA ticket? [y/n/o/q] (y=update existing, n=create new, o=open in browser, q=quit): ").strip().lower()
|
||||
ans = logged_input("Do you want to update this existing QA ticket? [y/n/o/q] (y=update existing, n=create new, o=open in browser, q=quit): ").strip().lower()
|
||||
if ans == 'y':
|
||||
args.update_qa = t.id
|
||||
log.info(f"Will update existing QA ticket #{t.id}.")
|
||||
@ -2077,14 +2106,14 @@ def build_branch(args):
|
||||
break
|
||||
elif ans == 'o':
|
||||
open_in_browser([t_url])
|
||||
print(f"Opened {t_url} in browser.")
|
||||
log.info(f"Opened {t_url} in browser.")
|
||||
elif ans == 'q':
|
||||
log.info("Exiting script.")
|
||||
sys.exit(0)
|
||||
elif ans == '':
|
||||
continue
|
||||
else:
|
||||
print("Invalid choice. Please enter y, n, o, or q.")
|
||||
log.info("Invalid choice. Please enter y, n, o, or q.")
|
||||
else:
|
||||
log.info("No open QA tickets found for this label. Will create a new QA ticket.")
|
||||
args.create_qa = True
|
||||
@ -2113,6 +2142,9 @@ def build_branch(args):
|
||||
if args.integration or merge_branch_name is False:
|
||||
merge_branch_name = branch
|
||||
|
||||
if args.log and not args.dry_run:
|
||||
add_file_log_handler(log, label=branch)
|
||||
|
||||
if base == 'HEAD':
|
||||
log.info("Branch base is HEAD; not checking out!")
|
||||
else:
|
||||
@ -2160,7 +2192,7 @@ def build_branch(args):
|
||||
|
||||
if not audit_passed and args.final_merge:
|
||||
log.error(f"Audit of PR #{pr} failed.")
|
||||
ans = input("Do you want to proceed with the final merge anyway? [y/N] ").strip().lower()
|
||||
ans = logged_input("Do you want to proceed with the final merge anyway? [y/N] ").strip().lower()
|
||||
if ans != 'y':
|
||||
log.error("Aborting final merge.")
|
||||
sys.exit(1)
|
||||
@ -2370,6 +2402,7 @@ def main():
|
||||
|
||||
group = parser.add_argument_group('General Options')
|
||||
group.add_argument('--debug', dest='debug', action='store_true', help='turn debugging on')
|
||||
group.add_argument('--log', dest='log', action='store_true', help='also log to <branch>.log (or ptl-tool.log if no branch) in the current working directory')
|
||||
group.add_argument('--dry-run', dest='dry_run', action='store_true', help='print actions without modifying remote state')
|
||||
group.add_argument('--examples', dest='examples', action='store_true', help='show extended examples and usage')
|
||||
group.add_argument('--git-dir', dest='git', action='store', default=git_dir, help='git directory')
|
||||
|
||||
@ -11,6 +11,7 @@ Run with:
|
||||
/tmp/ptl-test-venv/bin/pip install GitPython python-redmine requests pytest
|
||||
/tmp/ptl-test-venv/bin/pytest src/script/test_ptl_tool.py -v
|
||||
"""
|
||||
import builtins
|
||||
import importlib.util
|
||||
import logging
|
||||
import sys
|
||||
@ -136,3 +137,83 @@ def test_post_consolidated_review_noop_when_no_issues(ptl_tool):
|
||||
session = mock.Mock()
|
||||
report.post_consolidated_review(session, pr=12345, dry_run=False)
|
||||
session.post.assert_not_called()
|
||||
|
||||
|
||||
def test_log_flag_adds_filehandler(ptl_tool, tmp_path, monkeypatch):
|
||||
"""Without a label, the log file should be the generic ptl-tool.log in cwd."""
|
||||
logger = ptl_tool.log
|
||||
handlers_before = list(logger.handlers)
|
||||
monkeypatch.chdir(tmp_path)
|
||||
try:
|
||||
ret = ptl_tool.add_file_log_handler(logger)
|
||||
|
||||
file_handlers = [
|
||||
h for h in logger.handlers
|
||||
if isinstance(h, logging.FileHandler)
|
||||
]
|
||||
assert len(file_handlers) == 1
|
||||
expected = str(tmp_path / 'ptl-tool.log')
|
||||
assert file_handlers[0].baseFilename == expected
|
||||
assert ret == expected
|
||||
finally:
|
||||
logger.handlers = handlers_before
|
||||
|
||||
|
||||
def test_log_flag_uses_label_for_filename(ptl_tool, tmp_path, monkeypatch):
|
||||
"""When a label is provided, the log file should be <label>.log in cwd."""
|
||||
logger = ptl_tool.log
|
||||
handlers_before = list(logger.handlers)
|
||||
monkeypatch.chdir(tmp_path)
|
||||
try:
|
||||
ptl_tool.add_file_log_handler(logger, label='wip-bharath8-testing')
|
||||
|
||||
file_handlers = [
|
||||
h for h in logger.handlers
|
||||
if isinstance(h, logging.FileHandler)
|
||||
]
|
||||
assert len(file_handlers) == 1
|
||||
assert file_handlers[0].baseFilename == str(
|
||||
tmp_path / 'wip-bharath8-testing.log')
|
||||
finally:
|
||||
logger.handlers = handlers_before
|
||||
|
||||
|
||||
def test_prompt_logged_exactly_once(ptl_tool, tmp_path, monkeypatch):
|
||||
"""logged_input() must record the prompt exactly once in the log file."""
|
||||
logger = ptl_tool.log
|
||||
handlers_before = list(logger.handlers)
|
||||
monkeypatch.chdir(tmp_path)
|
||||
monkeypatch.setattr(builtins, "input", lambda prompt='': 'the-answer')
|
||||
try:
|
||||
log_path = ptl_tool.add_file_log_handler(logger, label="prompt-once")
|
||||
result = ptl_tool.logged_input("ask> ")
|
||||
assert result == "the-answer"
|
||||
finally:
|
||||
logger.handlers = handlers_before
|
||||
|
||||
contents = Path(log_path).read_text()
|
||||
assert "ask> the-answer" in contents
|
||||
assert contents.count("ask> ") == 1
|
||||
|
||||
|
||||
def test_logged_input_records_prompt_and_response_with_filehandler(ptl_tool, tmp_path, monkeypatch):
|
||||
"""logged_input() should capture both prompt text and typed response."""
|
||||
logger = ptl_tool.log
|
||||
handlers_before = list(logger.handlers)
|
||||
monkeypatch.chdir(tmp_path)
|
||||
monkeypatch.setattr(builtins, "input", lambda prompt='': 'user-answer')
|
||||
try:
|
||||
log_path = ptl_tool.add_file_log_handler(logger, label="capture")
|
||||
result = ptl_tool.logged_input("prompt> ")
|
||||
assert result == "user-answer"
|
||||
finally:
|
||||
logger.handlers = handlers_before
|
||||
|
||||
contents = Path(log_path).read_text()
|
||||
assert "prompt> user-answer" in contents
|
||||
|
||||
|
||||
def test_logged_input_without_filehandler_preserves_behavior(ptl_tool, monkeypatch):
|
||||
"""Without a FileHandler, logged_input() should behave like plain input()."""
|
||||
monkeypatch.setattr(builtins, "input", lambda prompt='': 'plain-answer')
|
||||
assert ptl_tool.logged_input("plain> ") == "plain-answer"
|
||||
|
||||
Loading…
Reference in New Issue
Block a user
