Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1742723003:
[Interpreter] Silence runtime errors in generate-bytecode-expectations. (Closed)
|
Created:
4 years, 10 months ago by Stefano Sanfilippo Modified:
4 years, 9 months ago CC:
v8-reviews_googlegroups.com, oth, rmcilroy Base URL:
https://chromium.googlesource.com/v8/v8.git@master Target Ref:
refs/pending/heads/master Project:
v8 Visibility:
Public. |
Description[Interpreter] Silence runtime errors in generate-bytecode-expectations.
Runtime errors will be suppressed in --rebaseline mode, unless the
--verbose flag is passed.
The reasoning behind (rebaseline && !verbose) and not just (verbose)
is to suppress harmless noise while updating the expectation for
existing, known good snippets, without hiding actually relevant
errors when the tool is used to write new expectation files.
In fact, some tests are supposed to produce a runtime error, which
might nevertheless alarm a developer who is just --rebaseline'ing.
BUG=v8:4280
LOG=N
Committed: https://crrev.com/dc71deb077a5e2cfe381aa0b8bbc4b9fe324d787
Cr-Commit-Position: refs/heads/master@{#34385}
Patch Set 1 #
Total comments: 2
Patch Set 2 : Make the suppression condition more readable. #Messages
Total messages: 14 (7 generated)
Description was changed from ========== [Interpreter] Silence runtime errors in generate-bytecode-expectations. Runtime errors will be suppressed in --rebaseline mode, unless the --verbose flag is passed. The reasoning behind this peculiar condition (rebaseline && !verbose) is to suppress harmless noise when updating the expectation for existing, known good snippets, while not hiding actually relevant errors when the tool is used to write new expectation files. In fact, some tests are supposed to produce a runtime error, which might nevertheless alarm a developer who is just rebaselining. On the other hand, we don't want to hide errors from people writing new tests, since these errors might be unwanted. BUG=v8:4280 LOG=N ========== to ========== [Interpreter] Silence runtime errors in generate-bytecode-expectations. Runtime errors will be suppressed in --rebaseline mode, unless the --verbose flag is passed. The reasoning behind (rebaseline && !verbose) and not just (verbose) is to suppress harmless noise while updating the expectation for existing, known good snippets, without not hiding actually relevant errors when the tool is used to write new expectation files. In fact, some tests are supposed to produce a runtime error, which might nevertheless alarm a developer who is just rebaselining. On the other hand, we don't want to hide errors from people writing new tests, since these errors might be unwanted. BUG=v8:4280 LOG=N ==========
ssanfilippo@chromium.org changed reviewers: + mstarzinger@chromium.org, oth@chromium.org, rmcilroy@chromium.org
As discussed, here is a patch to suppress error messages when rebaselining. PTAL.
Description was changed from ========== [Interpreter] Silence runtime errors in generate-bytecode-expectations. Runtime errors will be suppressed in --rebaseline mode, unless the --verbose flag is passed. The reasoning behind (rebaseline && !verbose) and not just (verbose) is to suppress harmless noise while updating the expectation for existing, known good snippets, without not hiding actually relevant errors when the tool is used to write new expectation files. In fact, some tests are supposed to produce a runtime error, which might nevertheless alarm a developer who is just rebaselining. On the other hand, we don't want to hide errors from people writing new tests, since these errors might be unwanted. BUG=v8:4280 LOG=N ========== to ========== [Interpreter] Silence runtime errors in generate-bytecode-expectations. Runtime errors will be suppressed in --rebaseline mode, unless the --verbose flag is passed. The reasoning behind (rebaseline && !verbose) and not just (verbose) is to suppress harmless noise while updating the expectation for existing, known good snippets, without hiding actually relevant errors when the tool is used to write new expectation files. In fact, some tests are supposed to produce a runtime error, which might nevertheless alarm a developer who is just --rebaseline'ing. BUG=v8:4280 LOG=N ==========
LGTM with one suggestion, thanks. https://codereview.chromium.org/1742723003/diff/1/test/cctest/interpreter/gen... File test/cctest/interpreter/generate-bytecode-expectations.cc (right): https://codereview.chromium.org/1742723003/diff/1/test/cctest/interpreter/gen... test/cctest/interpreter/generate-bytecode-expectations.cc:538: options.rebaseline() && !options.verbose() ? PrintMessage Could you pull the "options.rebaseline() && !options.verbose()" into a local variable to make this a bit more readable.
https://codereview.chromium.org/1742723003/diff/1/test/cctest/interpreter/gen... File test/cctest/interpreter/generate-bytecode-expectations.cc (right): https://codereview.chromium.org/1742723003/diff/1/test/cctest/interpreter/gen... test/cctest/interpreter/generate-bytecode-expectations.cc:538: options.rebaseline() && !options.verbose() ? PrintMessage On 2016/02/26 16:57:58, rmcilroy wrote: > Could you pull the "options.rebaseline() && !options.verbose()" into a local > variable to make this a bit more readable. Done.
LGTM.
The CQ bit was checked by ssanfilippo@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from rmcilroy@chromium.org Link to the patchset: https://codereview.chromium.org/1742723003/#ps20001 (title: "Make the suppression condition more readable.")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1742723003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1742723003/20001
Message was sent while issue was closed.
Description was changed from ========== [Interpreter] Silence runtime errors in generate-bytecode-expectations. Runtime errors will be suppressed in --rebaseline mode, unless the --verbose flag is passed. The reasoning behind (rebaseline && !verbose) and not just (verbose) is to suppress harmless noise while updating the expectation for existing, known good snippets, without hiding actually relevant errors when the tool is used to write new expectation files. In fact, some tests are supposed to produce a runtime error, which might nevertheless alarm a developer who is just --rebaseline'ing. BUG=v8:4280 LOG=N ========== to ========== [Interpreter] Silence runtime errors in generate-bytecode-expectations. Runtime errors will be suppressed in --rebaseline mode, unless the --verbose flag is passed. The reasoning behind (rebaseline && !verbose) and not just (verbose) is to suppress harmless noise while updating the expectation for existing, known good snippets, without hiding actually relevant errors when the tool is used to write new expectation files. In fact, some tests are supposed to produce a runtime error, which might nevertheless alarm a developer who is just --rebaseline'ing. BUG=v8:4280 LOG=N ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== [Interpreter] Silence runtime errors in generate-bytecode-expectations. Runtime errors will be suppressed in --rebaseline mode, unless the --verbose flag is passed. The reasoning behind (rebaseline && !verbose) and not just (verbose) is to suppress harmless noise while updating the expectation for existing, known good snippets, without hiding actually relevant errors when the tool is used to write new expectation files. In fact, some tests are supposed to produce a runtime error, which might nevertheless alarm a developer who is just --rebaseline'ing. BUG=v8:4280 LOG=N ========== to ========== [Interpreter] Silence runtime errors in generate-bytecode-expectations. Runtime errors will be suppressed in --rebaseline mode, unless the --verbose flag is passed. The reasoning behind (rebaseline && !verbose) and not just (verbose) is to suppress harmless noise while updating the expectation for existing, known good snippets, without hiding actually relevant errors when the tool is used to write new expectation files. In fact, some tests are supposed to produce a runtime error, which might nevertheless alarm a developer who is just --rebaseline'ing. BUG=v8:4280 LOG=N Committed: https://crrev.com/dc71deb077a5e2cfe381aa0b8bbc4b9fe324d787 Cr-Commit-Position: refs/heads/master@{#34385} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/dc71deb077a5e2cfe381aa0b8bbc4b9fe324d787 Cr-Commit-Position: refs/heads/master@{#34385} |
