|
|
Created:
4 years, 5 months ago by Alexander Alekseev Modified:
4 years, 4 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionChromeOS Web UI Tests: make WebUIBrowserTest error message more readable.
WebUIBrowserTests fail if any JS console errors are found. This CL
makes error mesages more readable in this case.
BUG=604119
TEST=none
Committed: https://crrev.com/b39d5a274015d54e191dccf8c2eaba490f8a875f
Cr-Commit-Position: refs/heads/master@{#405418}
Patch Set 1 #
Total comments: 1
Messages
Total messages: 21 (12 generated)
alemate@chromium.org changed reviewers: + phajdan.jr@chromium.org
Please review.
The CQ bit was checked by alemate@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
The CQ bit was checked by alemate@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org to run a CQ dry run
Dry run: This issue passed the CQ dry run.
LGTM, thanks!
The CQ bit was checked by alemate@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Committed patchset #1 (id:1)
Message was sent while issue was closed.
CQ bit was unchecked.
Message was sent while issue was closed.
Description was changed from ========== ChromeOS Web UI Tests: make WebUIBrowserTest error message more readable. WebUIBrowserTests fail if any JS console errors are found. This CL makes error mesages more readable in this case. BUG=604119 TEST=none ========== to ========== ChromeOS Web UI Tests: make WebUIBrowserTest error message more readable. WebUIBrowserTests fail if any JS console errors are found. This CL makes error mesages more readable in this case. BUG=604119 TEST=none Committed: https://crrev.com/b39d5a274015d54e191dccf8c2eaba490f8a875f Cr-Commit-Position: refs/heads/master@{#405418} ==========
Message was sent while issue was closed.
Patchset 1 (id:??) landed as https://crrev.com/b39d5a274015d54e191dccf8c2eaba490f8a875f Cr-Commit-Position: refs/heads/master@{#405418}
Message was sent while issue was closed.
michaelpg@chromium.org changed reviewers: + michaelpg@chromium.org
Message was sent while issue was closed.
https://codereview.chromium.org/2145493002/diff/1/chrome/test/base/web_ui_bro... File chrome/test/base/web_ui_browser_test.cc (right): https://codereview.chromium.org/2145493002/diff/1/chrome/test/base/web_ui_bro... chrome/test/base/web_ui_browser_test.cc:464: LOG(ERROR) << "JS ERROR: '" << msg << "'"; Do you not normally see console errors when your tests fail? I'm seeing duplicate errors now because they're logged first during the test, and then again at the end here. That's annoying IMO -- is this WAI?
Message was sent while issue was closed.
On 2016/07/29 22:38:29, michaelpg wrote: > https://codereview.chromium.org/2145493002/diff/1/chrome/test/base/web_ui_bro... > File chrome/test/base/web_ui_browser_test.cc (right): > > https://codereview.chromium.org/2145493002/diff/1/chrome/test/base/web_ui_bro... > chrome/test/base/web_ui_browser_test.cc:464: LOG(ERROR) << "JS ERROR: '" << msg > << "'"; > Do you not normally see console errors when your tests fail? I'm seeing > duplicate errors now because they're logged first during the test, and then > again at the end here. That's annoying IMO -- is this WAI? I usually expect the test failure message to include the exact problem. We have a lot of different warnings and JS messages during tests run, so I was very surprised that I couldn't understand from the test failure message what exactly was wrong. And I spent quite some time looking for the source of the message and finding out what the problem was. So I believe this is better than it was. But if you can make failure message to include the exact messages that lead to the problem, it could be great!
Message was sent while issue was closed.
On 2016/07/29 22:46:45, Alexander Alekseev wrote: > On 2016/07/29 22:38:29, michaelpg wrote: > > > https://codereview.chromium.org/2145493002/diff/1/chrome/test/base/web_ui_bro... > > File chrome/test/base/web_ui_browser_test.cc (right): > > > > > https://codereview.chromium.org/2145493002/diff/1/chrome/test/base/web_ui_bro... > > chrome/test/base/web_ui_browser_test.cc:464: LOG(ERROR) << "JS ERROR: '" << > msg > > << "'"; > > Do you not normally see console errors when your tests fail? I'm seeing > > duplicate errors now because they're logged first during the test, and then > > again at the end here. That's annoying IMO -- is this WAI? > > I usually expect the test failure message to include the exact problem. > We have a lot of different warnings and JS messages during tests run, so I was > very surprised that I couldn't understand from the test failure message what > exactly was wrong. And I spent quite some time looking for the source of the > message and finding out what the problem was. > > So I believe this is better than it was. But if you can make failure message to > include the exact messages that lead to the problem, it could be great! Can you give an example of a test that this improves? |