58 bytes, text/x-review-board-request
I have done an updated try run to attempt enabling promise rejection tracking in bug 1240804. There still appear to be some unresolved rule view failures: devtools/client/inspector/rules/test/browser_rules_add-property_01.js devtools/client/inspector/rules/test/browser_rules_colorpicker-commit-on-ENTER.js devtools/client/inspector/rules/test/browser_rules_colorpicker-hides-on-tooltip.js devtools/client/inspector/rules/test/browser_rules_colorpicker-revert-on-ESC.js devtools/client/inspector/rules/test/browser_rules_completion-existing-property_02.js devtools/client/inspector/rules/test/browser_rules_completion-new-property_02.js devtools/client/inspector/rules/test/browser_rules_completion-new-property_04.js devtools/client/inspector/rules/test/browser_rules_cubicbezier-revert-on-ESC.js devtools/client/inspector/rules/test/browser_rules_cubicbezier-revert-on-ESC.js devtools/client/inspector/rules/test/browser_rules_edit-property_04.js devtools/client/inspector/rules/test/browser_rules_edit-property_04.js devtools/client/inspector/rules/test/browser_rules_edit-property-cancel.js devtools/client/inspector/rules/test/browser_rules_edit-property-cancel.js devtools/client/inspector/rules/test/browser_rules_edit-property-commit.js devtools/client/inspector/rules/test/browser_rules_edit-property-commit.js devtools/client/inspector/rules/test/browser_rules_eyedropper.js devtools/client/inspector/rules/test/browser_rules_eyedropper.js devtools/client/inspector/rules/test/browser_rules_livepreview.js devtools/client/inspector/rules/test/browser_rules_livepreview.js devtools/client/inspector/rules/test/browser_rules_livepreview.js devtools/client/inspector/rules/test/browser_rules_multiple_properties_02.js devtools/client/inspector/rules/test/browser_rules_multiple_properties_02.js devtools/client/inspector/rules/test/browser_rules_multiple_properties_02.js Remember to apply the patch from bug 1240804 to see the failures. More details available at: https://docs.google.com/spreadsheets/d/1ZKXW9HqnxJNdeEVG7s33_TGSV94PdUJCsQsZe1GGqjY/edit
Working on massive patch series in bug 1246677 that aims at getting rid of CPOW usage in rule-view tests. I ended up cleaning more in that bug that just CPOW usages, including getting rid of some promise rejections.
Depends on: 1246677
If I apply the patches from bug 1246677 *and* run each test from comment 0 individually, with e10s, then only the following ones still fail: devtools/client/inspector/rules/test/browser_rules_completion-existing-property_02.js devtools/client/inspector/rules/test/browser_rules_completion-new-property_02.js devtools/client/inspector/rules/test/browser_rules_edit-property-commit.js devtools/client/inspector/rules/test/browser_rules_multiple_properties_02.js They should be pretty easy to fix. I still need to run the whole test suite though as that might impact the result, and try without e10s too.
Review commit: https://reviewboard.mozilla.org/r/34785/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/34785/
Attachment #8718892 - Flags: review?(jryans)
Sorry jryans for picking you for this, feel free to forward to someone else if you don't want to review code changes in this part of the codebase. Hopefully this is simple enough (and test-only). The basic thing here is waiting for the right events at the right time ("ruleview-changed"). I have already inundated my usual go-to reviewers for the rule-view in bug 1246677
Comment on attachment 8718892 [details] MozReview Request: Bug 1247263 - Round 2 of fixing unhandled rejected promises in ruleview tests :tromey said he'll take a look at this.
Attachment #8718892 - Flags: review?(jryans) → review?(ttromey)
Comment on attachment 8718892 [details] MozReview Request: Bug 1247263 - Round 2 of fixing unhandled rejected promises in ruleview tests https://reviewboard.mozilla.org/r/34785/#review31421 Looks great, thanks for doing this.
Attachment #8718892 - Flags: review?(ttromey) → review+
Sorry for the delay here. I know this is R+ but it depends on bug 1246677 which itself depending on another bug that took time to land. I finally landed bug 1246677 but it got backed out. I'm working on a fix though. In the meantime, I have a green try for the changes here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=13dc6c29fb82&group_state=expanded But pushing to try with the patch in bug 1240804 applied (to fail on unhandled promise rejections) still shows 3 failures in rule-view: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fc2024973bc1&group_state=expanded Everything was passing fine locally last time I tried. So I'll run these again and try and fix the remaining issues.
I was able to fix 2 of the 3 remaining failures. I can't reproduce the last one locally. So, since this patch has been around for some time, is R+ and is green on try, I will land it as is, and we can file a new bug to fix the remaining failures with bug 1240804 applied, if any.
You need to log in before you can comment on or make changes to this bug.