-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[Debugify] Add 'acceptance-test' mode for the debugify report script #147574
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
e15f992
8a9e435
2f5b32f
733a3f2
e5b6c36
4fddfaf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,17 @@ | ||
| RUN: %llvm-original-di-preservation %p/Inputs/sample.json %t.html | FileCheck %s | ||
| RUN: %llvm-original-di-preservation %p/Inputs/sample.json --report-file %t.html | FileCheck %s | ||
| RUN: diff -w %p/Inputs/expected-sample.html %t.html | ||
| CHECK: The {{.+}}.html generated. | ||
| CHECK-NOT: Skipped lines: | ||
|
|
||
| RUN: %llvm-original-di-preservation %p/Inputs/corrupted.json %t2.html | FileCheck %s -check-prefix=CORRUPTED | ||
| RUN: %llvm-original-di-preservation %p/Inputs/corrupted.json --report-file %t2.html | FileCheck %s -check-prefix=CORRUPTED | ||
| RUN: diff -w %p/Inputs/expected-skipped.html %t2.html | ||
| CORRUPTED: Skipped lines: 3 | ||
| CORRUPTED: Skipped bugs: 1 | ||
|
|
||
| RUN: %llvm-original-di-preservation -compress %p/Inputs/sample.json %t3.html | FileCheck %s -check-prefix=COMPRESSED | ||
| RUN: %llvm-original-di-preservation --compress %p/Inputs/sample.json --report-file %t3.html | FileCheck %s -check-prefix=COMPRESSED | ||
| RUN: diff -w %p/Inputs/expected-compressed.html %t3.html | ||
| COMPRESSED: The {{.+}}.html generated. | ||
| COMPRESSED-NOT: Skipped lines: | ||
|
|
||
| RUN: %llvm-original-di-preservation %p/Inputs/origin.json %t4.html | FileCheck %s | ||
| RUN: %llvm-original-di-preservation %p/Inputs/origin.json --report-file %t4.html | FileCheck %s | ||
| RUN: diff -w %p/Inputs/expected-origin.html %t4.html |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| RUN: not %llvm-original-di-preservation %p/Inputs/sample.json --error-test | FileCheck %s | ||
| CHECK: DILocation Bugs: | ||
| CHECK-NEXT: test.ll: | ||
| CHECK-NEXT: no-name: | ||
| CHECK-NEXT: - action: not-generate | ||
| CHECK-NEXT: bb_name: no-name | ||
| CHECK-NEXT: fn_name: fn | ||
| CHECK-NEXT: instr: extractvalue | ||
| CHECK-NEXT: - action: not-generate | ||
| CHECK-NEXT: bb_name: no-name | ||
| CHECK-NEXT: fn_name: fn | ||
| CHECK-NEXT: instr: insertvalue | ||
| CHECK-NEXT: - action: not-generate | ||
| CHECK-NEXT: bb_name: no-name | ||
| CHECK-NEXT: fn_name: fn1 | ||
| CHECK-NEXT: instr: insertvalue | ||
| CHECK-NEXT: - action: not-generate | ||
| CHECK-NEXT: bb_name: no-name | ||
| CHECK-NEXT: fn_name: fn1 | ||
| CHECK-NEXT: instr: extractvalue | ||
| CHECK: Errors detected for: | ||
|
|
||
| RUN: not %llvm-original-di-preservation %p/Inputs/sample.json --error-test --compress | FileCheck %s --check-prefix=COMPRESS | ||
| COMPRESS: DILocation Bugs: | ||
| COMPRESS-NEXT: test.ll: | ||
| COMPRESS-NEXT: no-name: | ||
| COMPRESS-NEXT: - action: not-generate | ||
| COMPRESS-NEXT: bb_name: no-name | ||
| COMPRESS-NEXT: fn_name: fn | ||
| COMPRESS-NEXT: instr: extractvalue | ||
| COMPRESS-NEXT: - action: not-generate | ||
| COMPRESS-NEXT: bb_name: no-name | ||
| COMPRESS-NEXT: fn_name: fn | ||
| COMPRESS-NEXT: instr: insertvalue | ||
| COMPRESS: Errors detected for: | ||
|
|
||
| RUN: %llvm-original-di-preservation %p/Inputs/non-existent.json --error-test | FileCheck %s --check-prefix=EMPTY | ||
| EMPTY: No errors detected for: |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,7 +11,6 @@ | |
| from collections import defaultdict | ||
| from collections import OrderedDict | ||
|
|
||
|
|
||
| class DILocBug: | ||
| def __init__(self, origin, action, bb_name, fn_name, instr): | ||
| self.origin = origin | ||
|
|
@@ -20,28 +19,77 @@ def __init__(self, origin, action, bb_name, fn_name, instr): | |
| self.fn_name = fn_name | ||
| self.instr = instr | ||
|
|
||
| def __str__(self): | ||
| def key(self): | ||
| return self.action + self.bb_name + self.fn_name + self.instr | ||
|
|
||
| def to_dict(self): | ||
| result = { | ||
| "instr": self.instr, | ||
| "fn_name": self.fn_name, | ||
| "bb_name": self.bb_name, | ||
| "action": self.action, | ||
| } | ||
| if self.origin: | ||
| result["origin"] = self.origin | ||
| return result | ||
|
|
||
|
|
||
| class DISPBug: | ||
| def __init__(self, action, fn_name): | ||
| self.action = action | ||
| self.fn_name = fn_name | ||
|
|
||
| def __str__(self): | ||
| def key(self): | ||
| return self.action + self.fn_name | ||
|
|
||
| def to_dict(self): | ||
| return { | ||
| "fn_name": self.fn_name, | ||
| "action": self.action, | ||
| } | ||
|
|
||
|
|
||
| class DIVarBug: | ||
| def __init__(self, action, name, fn_name): | ||
| self.action = action | ||
| self.name = name | ||
| self.fn_name = fn_name | ||
|
|
||
| def __str__(self): | ||
| def key(self): | ||
| return self.action + self.name + self.fn_name | ||
|
|
||
| def to_dict(self): | ||
| return { | ||
| "fn_name": self.fn_name, | ||
| "name": self.name, | ||
| "action": self.action, | ||
| } | ||
|
|
||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this becomes load-bearing, at some point we're going to have to switch over to using the "proper" yaml dumping and pretty-printing facilities. Does it make sense to do that now instead? (I don't think we need to make an all-singing-all-dancing implementation for all scripts, we can just focus on their narrow purpose).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I originally started using "proper" YAML, but this does require an extra package. Currently this (and most other utility scripts) can be run with just python, with no install commands necessary, and I consider preserving this to be a benefit worth some cost. My expectation is, this script has very straightforward and limited functionality, so there isn't too much to gain from using a proper YAML package - only very basic printing logic is required, and the YAML dumping segment is purely for user benefit (it has no expectation of being programmatically parsed). |
||
| def print_bugs_yaml(name, bugs_dict, indent=2): | ||
| def get_bug_line(indent_level: int, text: str, margin_mark: bool = False): | ||
| if margin_mark: | ||
| return "- ".rjust(indent_level * indent) + text | ||
| return " " * indent * indent_level + text | ||
|
|
||
| print(f"{name}:") | ||
| for bugs_file, bugs_pass_dict in sorted(iter(bugs_dict.items())): | ||
| print(get_bug_line(1, f"{bugs_file}:")) | ||
| for bugs_pass, bugs_list in sorted(iter(bugs_pass_dict.items())): | ||
| print(get_bug_line(2, f"{bugs_pass}:")) | ||
| for bug in bugs_list: | ||
| bug_dict = bug.to_dict() | ||
| first_line = True | ||
| # First item needs a '-' in the margin. | ||
| for key, val in sorted(iter(bug_dict.items())): | ||
| if "\n" in val: | ||
| # Output block text for any multiline string. | ||
| print(get_bug_line(3, f"{key}: |", first_line)) | ||
| for line in val.splitlines(): | ||
| print(get_bug_line(4, line)) | ||
| else: | ||
| print(get_bug_line(3, f"{key}: {val}", first_line)) | ||
| first_line = False | ||
|
|
||
| # Report the bugs in form of html. | ||
| def generate_html_report( | ||
|
|
@@ -430,9 +478,16 @@ def get_json_chunk(file, start, size): | |
| # Parse the program arguments. | ||
| def parse_program_args(parser): | ||
| parser.add_argument("file_name", type=str, help="json file to process") | ||
| parser.add_argument("html_file", type=str, help="html file to output data") | ||
| parser.add_argument( | ||
| "-compress", action="store_true", help="create reduced html report" | ||
| parser.add_argument("--compress", action="store_true", help="create reduced report") | ||
|
||
|
|
||
| report_type_group = parser.add_mutually_exclusive_group(required=True) | ||
| report_type_group.add_argument( | ||
| "--report-file", type=str, help="output HTML file for the generated report" | ||
|
||
| ) | ||
| report_type_group.add_argument( | ||
| "--error-test", | ||
| action="store_true", | ||
| help="if set, produce terminal-friendly output and return 0 iff the input file is empty or does not exist", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, it does make sense. |
||
| ) | ||
|
|
||
| return parser.parse_args() | ||
|
|
@@ -442,10 +497,20 @@ def Main(): | |
| parser = argparse.ArgumentParser() | ||
| opts = parse_program_args(parser) | ||
|
|
||
| if not opts.html_file.endswith(".html"): | ||
| if opts.report_file is not None and not opts.report_file.endswith(".html"): | ||
| print("error: The output file must be '.html'.") | ||
| sys.exit(1) | ||
|
|
||
| if opts.error_test: | ||
| if os.path.isdir(opts.file_name): | ||
| print(f"error: Directory passed as input file: '{opts.file_name}'") | ||
| sys.exit(1) | ||
| if not os.path.exists(opts.file_name): | ||
| # We treat an empty input file as a success, as debugify will generate an output file iff any errors are | ||
| # found, meaning we expect 0 errors to mean that the expected file does not exist. | ||
| print(f"No errors detected for: {opts.file_name}") | ||
| sys.exit(0) | ||
|
||
|
|
||
| # Use the defaultdict in order to make multidim dicts. | ||
| di_location_bugs = defaultdict(lambda: defaultdict(list)) | ||
| di_subprogram_bugs = defaultdict(lambda: defaultdict(list)) | ||
|
|
@@ -489,9 +554,9 @@ def Main(): | |
| skipped_lines += 1 | ||
| continue | ||
|
|
||
| di_loc_bugs = di_location_bugs[bugs_file][bugs_pass] | ||
| di_sp_bugs = di_subprogram_bugs[bugs_file][bugs_pass] | ||
| di_var_bugs = di_variable_bugs[bugs_file][bugs_pass] | ||
| di_loc_bugs = di_location_bugs.get("bugs_file", {}).get("bugs_pass", []) | ||
| di_sp_bugs = di_subprogram_bugs.get("bugs_file", {}).get("bugs_pass", []) | ||
| di_var_bugs = di_variable_bugs.get("bugs_file", {}).get("bugs_pass", []) | ||
|
|
||
| # Omit duplicated bugs. | ||
| di_loc_set = set() | ||
|
|
@@ -515,8 +580,8 @@ def Main(): | |
| skipped_bugs += 1 | ||
| continue | ||
| di_loc_bug = DILocBug(origin, action, bb_name, fn_name, instr) | ||
| if not str(di_loc_bug) in di_loc_set: | ||
| di_loc_set.add(str(di_loc_bug)) | ||
| if not di_loc_bug.key() in di_loc_set: | ||
| di_loc_set.add(di_loc_bug.key()) | ||
| if opts.compress: | ||
| pass_instr = bugs_pass + instr | ||
| if not pass_instr in di_loc_pass_instr_set: | ||
|
|
@@ -538,8 +603,8 @@ def Main(): | |
| skipped_bugs += 1 | ||
| continue | ||
| di_sp_bug = DISPBug(action, name) | ||
| if not str(di_sp_bug) in di_sp_set: | ||
| di_sp_set.add(str(di_sp_bug)) | ||
| if not di_sp_bug.key() in di_sp_set: | ||
| di_sp_set.add(di_sp_bug.key()) | ||
| if opts.compress: | ||
| pass_fn = bugs_pass + name | ||
| if not pass_fn in di_sp_pass_fn_set: | ||
|
|
@@ -562,8 +627,8 @@ def Main(): | |
| skipped_bugs += 1 | ||
| continue | ||
| di_var_bug = DIVarBug(action, name, fn_name) | ||
| if not str(di_var_bug) in di_var_set: | ||
| di_var_set.add(str(di_var_bug)) | ||
| if not di_var_bug.key() in di_var_set: | ||
| di_var_set.add(di_var_bug.key()) | ||
| if opts.compress: | ||
| pass_var = bugs_pass + name | ||
| if not pass_var in di_var_pass_var_set: | ||
|
|
@@ -582,19 +647,40 @@ def Main(): | |
| skipped_bugs += 1 | ||
| continue | ||
|
|
||
| di_location_bugs[bugs_file][bugs_pass] = di_loc_bugs | ||
| di_subprogram_bugs[bugs_file][bugs_pass] = di_sp_bugs | ||
| di_variable_bugs[bugs_file][bugs_pass] = di_var_bugs | ||
|
|
||
| generate_html_report( | ||
| di_location_bugs, | ||
| di_subprogram_bugs, | ||
| di_variable_bugs, | ||
| di_location_bugs_summary, | ||
| di_sp_bugs_summary, | ||
| di_var_bugs_summary, | ||
| opts.html_file, | ||
| ) | ||
| if di_loc_bugs: | ||
| di_location_bugs[bugs_file][bugs_pass] = di_loc_bugs | ||
| if di_sp_bugs: | ||
| di_subprogram_bugs[bugs_file][bugs_pass] = di_sp_bugs | ||
| if di_var_bugs: | ||
| di_variable_bugs[bugs_file][bugs_pass] = di_var_bugs | ||
|
|
||
| if opts.report_file is not None: | ||
| generate_html_report( | ||
| di_location_bugs, | ||
| di_subprogram_bugs, | ||
| di_variable_bugs, | ||
| di_location_bugs_summary, | ||
| di_sp_bugs_summary, | ||
| di_var_bugs_summary, | ||
| opts.report_file, | ||
| ) | ||
| else: | ||
| # Pretty(ish) print the detected bugs, but check if any exist first so that we don't print an empty dict. | ||
| if di_location_bugs: | ||
| print_bugs_yaml('DILocation Bugs', di_location_bugs) | ||
| if di_subprogram_bugs: | ||
| print_bugs_yaml('DISubprogram Bugs', di_subprogram_bugs) | ||
| if di_variable_bugs: | ||
| print_bugs_yaml('DIVariable Bugs', di_variable_bugs) | ||
|
|
||
| if opts.error_test: | ||
| if any((di_location_bugs, di_subprogram_bugs, di_variable_bugs)): | ||
| # Add a newline gap after printing at least one error. | ||
| print() | ||
| print(f"Errors detected for: {opts.file_name}") | ||
| sys.exit(1) | ||
| else: | ||
| print(f"No errors detected for: {opts.file_name}") | ||
|
|
||
| if skipped_lines > 0: | ||
| print("Skipped lines: " + str(skipped_lines)) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No test coverage for this property ("origin")?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added in latest commit.