New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 780589 link

Starred by 2 users

Issue metadata

Status: Assigned
Owner:
Last visit > 30 days ago
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

WebUIBrowserTest does not detect javascript errors from "preloaded" libraries

Project Member Reported by lukasza@chromium.org, Nov 1 2017

Issue description

WebUIBrowserTest supports preloading javascript libraries.  On the renderer side, the corresponding scripts end up stored and executed (around commit time) from ChromeRenderFrameObserver::webui_javascript_.  If such scripts report javascript errors, the errors are ignored.

Example where javascript errors happen, but tests pass:

    $ cat out/cros/args.gn 
    dcheck_always_on = true
    is_component_build = true
    is_debug = false
    target_os = "chromeos"
    use_goma = true

    $ ninja -C out/cros -j 1500 -l 25 chromevox_tests browser_tests
    ...

    $ DISPLAY=:20 out/cros/chromevox_tests --gtest_filter=*CvoxBrailleIntegrationUnitTest.WriteWithSpans*
    ...
    [115664:115664:1101/133609.258899:INFO:CONSOLE(153)] "Uncaught TypeError: Cannot read property 'getURL' of undefined", source: earcon_engine.js (153)
    [115664:115664:1101/133609.261122:INFO:CONSOLE(14)] "Uncaught TypeError: Cannot read property 'AutomationNode' of undefined", source: recovery_strategy.js (14)
    [115664:115664:1101/133609.263432:INFO:CONSOLE(16)] "Uncaught TypeError: Cannot read property 'AutomationNode' of undefined", source: automation_predicate.js (16)
    [115664:115664:1101/133609.264543:INFO:CONSOLE(21)] "Uncaught TypeError: Cannot read property 'AutomationNode' of undefined", source: automation_util.js (21)
    [115664:115664:1101/133609.266805:INFO:CONSOLE(65)] "Uncaught TypeError: Cannot read property 'AutomationNode' of undefined", source: cursors.js (65)
    [115664:115664:1101/133609.286202:INFO:CONSOLE(28)] "Uncaught TypeError: Cannot read property 'AutomationNode' of undefined", source: output.js (28)
    [115664:115664:1101/133609.421675:INFO:CONSOLE(1220)] "Running TestCase CvoxBrailleIntegrationUnitTest.WriteWithSpans", source: test_api.js (1220)
    ...
    [       OK ] CvoxBrailleIntegrationUnitTest.WriteWithSpans (2210 ms)
    [----------] 1 test from CvoxBrailleIntegrationUnitTest (2210 ms total)
    [----------] Global test environment tear-down
    [==========] 1 test from 1 test case ran. (2210 ms total)
    [  PASSED  ] 1 test.

    (the example is CrOS-only [sorry, that's the example what I run into] but based on reading the code it seems that the problem is OS-agnostic)


chrome/test/base/web_ui_browser_test.cc tries to detect javascript errors, by intercepting logs via base/logging.h, filtering out non-javascript logs and storing the errors in a global variable:

    base::LazyInstance<std::vector<std::string>>::DestructorAtExit error_messages_ =
        LAZY_INSTANCE_INITIALIZER;
    
    // Intercepts all log messages.
    bool LogHandler(int severity,
                    const char* file,
                    int line,
                    size_t message_start,
                    const std::string& str) {
      if (severity == logging::LOG_ERROR && file &&
          std::string("CONSOLE") == file) {
        error_messages_.Get().push_back(str);
      }

      return false;
    }

For some reason the errors from "preloaded" libraries do not impact test results.
 
Another problem with error handling is that errors from "preloaded" libraries can be incorrectly attributed to individual tests.  This happens because:

1. "preloaded" libraries are async / fire-and-forget

2. errors from "preloaded" libraries are retained
   (i.e. stored errors are not cleared when starting to execute a javascript test)

This is a separate issue and I plan to fix this as part of the work to fix issue 766329 as follows:

    --- a/chrome/test/base/web_ui_browser_test.cc
    +++ b/chrome/test/base/web_ui_browser_test.cc
    @@ -461,6 +461,7 @@ bool WebUIBrowserTest::RunJavascriptUsingHandler(
         SetupHandlers();
 
       bool result = true;
    +  error_messages_.Get().clear();
 
       for (size_t i = 0; i < libraries.size(); ++i)
         test_handler_->PreloadJavaScript(libraries[i], preload_host);
    @@ -480,7 +481,6 @@ bool WebUIBrowserTest::RunJavascriptUsingHandler(
         LOG(ERROR) << "JS call assumed failed, because JS console error(s) found.";
     
         result = false;
    -    error_messages_.Get().clear();
       }
       return result;
     }

The fix above will ensure that executing a javascript test will start with a blank slate / without being polluted by errors from previous javascript executions.
Components: UI>Browser>WebUI
Owner: groby@chromium.org
groby@ - could you please help triage this WebUI Test issue?
Status: Assigned (was: Untriaged)

Sign in to add a comment