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
bug 1383113: switch from getattr to getitem notation in breakpad rules #5355
bug 1383113: switch from getattr to getitem notation in breakpad rules #5355
Conversation
This switches from getattr notation to getitem notation in the breakpad processor rules. We're transitioning off of DotDict and dict doesn't support getattr notation.
or processed_crash.cpu_arch != "x86" | ||
processed_crash.get("product", "") != "Firefox" | ||
or not processed_crash.get("os_name", "").startswith("Windows") | ||
or processed_crash.get("cpu_arch", "") != "x86" |
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.
I'll add a note to myself to ask Gabriele and find out if this is still true. It might not be.
) | ||
if frames and frames[0].get("module", False): | ||
# there is a module at the top of the stack, we don't want this | ||
return False | ||
|
||
signature = processed_crash.get("signature", "") |
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.
This rule needs to run after mdsw and after signature generation. That should be denoted in the class docstring.
@@ -464,28 +449,34 @@ def test_success_all_types_of_signatures(self, mocked_subprocess_module): | |||
"Small | js::irregexp::ExecuteCode<T>", | |||
] | |||
for signature in signatures: | |||
processed_crash = DotDict(base_processed_crash) | |||
processed_crash.signature = signature | |||
processed_crash = copy.copy(base_processed_crash) |
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.
All these copies need to be deepcopies because the rules are changing the processed crash in deep and existentially meaningful ways.
The processed crash is deep and wonderful and doing a shallow copy means that tests may not be testing what they think they're testing.
self-r+ |
This switches from getattr notation to getitem notation in the breakpad processor rules. We're transitioning off of DotDict and dict doesn't support getattr notation.
I did this in a few passes:
breakpad.py
file and get tests to passDotDict
usage in thetest_breakpad.py
file and get tests to passTo test: