Skip to content
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

Jump to bottom

Warnings when running in CSP mode #15852

Closed
peter-ahe-google opened this issue Jan 2, 2014 · 5 comments
Closed

Warnings when running in CSP mode #15852

peter-ahe-google opened this issue Jan 2, 2014 · 5 comments
Labels
area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. closed-obsolete Closed as the reported issue is no longer relevant type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)

Comments

@peter-ahe-google
Copy link
Contributor

Running

  ./tools/test.py -mrelease -cdart2js --time -pcolor --report --failure-summary -rdrt --csp

produces warnings (see below).

I do not understand the benefit of warning on the console but not marking a test as failing. Most people don't test --csp locally, and will never see these warnings.

Please do not reassign this bug to dart2js. I'm not reporting a bug in dart2js, I'm reporting an issue in the test runner (warnings, not test failures).

[01:25 | 4% | + 125 | - 0]2014-01-02 13:32:12.504 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/pkg_polymer_example_canonicalization_test_canonicalization_test/test/canonicalization_test.html?crossOriginPort=64001)
2014-01-02 13:32:12.509 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/pkg_polymer_example_canonicalization_test_canonicalization_test/test/canonicalization_test.html?crossOriginPort=64001)
[01:26 | 4% | + 126 | - 0]2014-01-02 13:32:12.509 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/pkg_polymer_example_canonicalization_test_canonicalization_test/test/canonicalization_test.html?crossOriginPort=64001)
[02:14 | 7% | + 209 | - 0]2014-01-02 13:32:59.942 Warning: HttpServer: could not find file for request path: "/root_build/generated_tests/dart2js-drt-csp/pkg_template_binding_test_template_binding_test/pic.jpg"
[02:44 | 9% | + 280 | - 0]2014-01-02 13:33:31.014 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/samples_swarm_test_swarm_ui_lib_touch_touch_test/test.html?crossOriginPort=64001)
2014-01-02 13:33:31.015 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/samples_swarm_test_swarm_ui_lib_touch_touch_test/test.html?crossOriginPort=64001)
[02:45 | 9% | + 281 | - 0]2014-01-02 13:33:31.016 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/samples_swarm_test_swarm_ui_lib_touch_touch_test/test.html?crossOriginPort=64001)
[02:50 | 10% | + 297 | - 0]2014-01-02 13:33:36.323 Warning: HttpServer: could not find file for request path: "/root_build/generated_tests/dart2js-drt-csp/samples_swarm_test_swarm_test/data/Test0_2_1.html"
[04:06 | 18% | + 545 | - 0]2014-01-02 13:34:52.765 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_custom_created_callback_test/test.html?crossOriginPort=64001)
2014-01-02 13:34:52.766 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_custom_created_callback_test/test.html?crossOriginPort=64001)
[04:06 | 18% | + 546 | - 0]2014-01-02 13:34:52.767 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_custom_created_callback_test/test.html?crossOriginPort=64001)
[04:08 | 18% | + 559 | - 0]2014-01-02 13:35:03.009 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_custom_js_custom_test/test.html?crossOriginPort=64001)
2014-01-02 13:35:03.010 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_custom_js_custom_test/test.html?crossOriginPort=64001)
[04:17 | 18% | + 560 | - 0]2014-01-02 13:35:03.011 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_custom_js_custom_test/test.html?crossOriginPort=64001)
[04:27 | 19% | + 577 | - 0]2014-01-02 13:35:13.506 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_event_customevent_test/test.html?crossOriginPort=64001)
2014-01-02 13:35:13.507 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_event_customevent_test/test.html?crossOriginPort=64001)
[04:27 | 19% | + 578 | - 0]2014-01-02 13:35:13.520 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_event_customevent_test/test.html?crossOriginPort=64001)
[05:37 | 25% | + 761 | - 0]2014-01-02 13:36:23.774 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_webgl_1_test/test.html?crossOriginPort=64001&group=functional)
2014-01-02 13:36:23.775 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_webgl_1_test/test.html?crossOriginPort=64001&group=functional)
2014-01-02 13:36:23.781 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_webgl_1_test/test.html?crossOriginPort=64001&group=functional)
2014-01-02 13:36:23.781 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_webgl_1_test/test.html?crossOriginPort=64001&group=functional)
[05:37 | 25% | + 762 | - 0]2014-01-02 13:36:23.783 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_webgl_1_test/test.html?crossOriginPort=64001&group=functional)
[05:38 | 25% | + 764 | - 0]2014-01-02 13:36:23.966 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_wheelevent_test/test.html?crossOriginPort=64001)
2014-01-02 13:36:23.966 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_wheelevent_test/test.html?crossOriginPort=64001)
[05:38 | 25% | + 765 | - 0]2014-01-02 13:36:23.967 Warning: Test had 'FAIL' and 'PASS' in stdout. ("/Users/ahe/Dart/all/dart//client/tests/drt/Content Shell.app/Contents/MacOS/Content Shell" --no-timeout --dump-render-tree http://127.0.0.1:64000/root_build/generated_tests/dart2js-drt-csp/tests_html_wheelevent_test/test.html?crossOriginPort=64001)
[05:38 | 25% | + 768 | - 0]2014-01-02 13:36:24.489 Warning: HttpServer: could not find file for request path: "/root_build/generated_tests/dart2js-drt-csp/tests_html_worker_test/worker.js"
[05:38 | 25% | + 769 | - 0]2014-01-02 13:36:24.862 Warning: HttpServer: could not find file for request path: "/root_build/generated_tests/dart2js-drt-csp/tests_html_xhr_cross_origin_test/does_not_exist"
[08:34 | 52% | + 1558 | - 0]
@mkustermann
Copy link
Member

If test.dart sees FAIL and PASS it will report the test as FAIL'ed (the warning is just printed to surface this situation).

  bool get _browserTestFailure {
    ...
    if (containsFail && containsPass) {
      DebugLogger.warning("Test had 'FAIL' and 'PASS' in stdout. ($command)");
    }
    ...
    if (containsFail) {
      return true;
    }
    ...
   }

Looking at html/custom/js_custom_test for example, we can see it was marked as failing on dart2js-csp in the html.status file.

Now we come to the reason why there can be FAIL and PASS in the output:

* Our normal contract with tests is:

  • if the test is synchronous: if main is done and there were no exceptions/... we mark the test as PASSing
  • if the test is asynchronous: the test has to call 'print("unittest-suite-wait-for-done")' before main() is done and call 'print("unittest-suite-success")' or 'print("unittest-suite-fail")' [posting these messages to the window object instead of using print() works as well]

test_controller.js will intercept these messages and is able to correctly mark the test as passing or failing.

  • On all "normal" browsers (i.e. non content_shell) test.dart uses the browser controller. test_controller.js will send the result back to test.dart via a JSON interface.
  • In the specific case of content_shell, test_controller.js will print the result PASS or FAIL to the DOM (since the DOM is the only information test.dart gets from content_shell).

* BUT we have some tests using the unittest framework (in particular tests/html/*). The unittest framework does not use "unittest-suite-success/unittest-suite-fail" when a test is done.
What it does instead is posting "unittest-suite-done" and printing PASS/FAIL messages to the DOM, so test_controller.js knows only that the test is done. It assumes that the test passed if no exceptions/... occured.

  • Once again, when using content_shell, test_controller.js prints 'PASS' to the DOM in that case.

Since we know that we have this (incorrect) behavior from unittest, test.dart will grep for FAIL in the DOM (both when using content_shell and when browser controller) and mark the test as failing when a 'FAIL' was found in the DOM.

[As can be seen in the output above, the corresponding tests are all from the "html" test suite. All of these tests use the unittest framework.]

To summarize, there are two issues:
a) unittest framework is not using "unittest-suite-{success,fail}" but "unittest-suite-done" (so we have to rely on PASS/FAIL being printed to the DOM by the unittest framework)

b) when running tests inside content_shell the only information test.dart gets currently is the DOM, wo we have to use it as communication channel (i.e. test_controller.js has to print the outcome to the DOM).

The fundamental issue is a).

I've tried to solve it by changing unittest framework (see https://codereview.chromium.org/75393002/diff/110001/pkg/unittest/lib/html_config.dart), but I was told to not commit this CL.
Last time we had visitors from SEA we had discussions about this as well, without an agreement so far.

@peter-ahe-google
Copy link
Contributor Author

@martin:

The unittest frameworks throws an exception when a test fails and does not print PASS. This should be sufficient for test_controller.js.

I think you're saying that is not the case anymore, and then something must have regressed in either test_controller.js or unittest.

@mkustermann
Copy link
Member

@ahe:

I think it works roughly like this:

  • all async callbacks inside test() {...} are wrapped using expectAsync1()/...
  • calls to expect() throw a TestFailure exception
  • the wrappers catch this exception and will record the failure in an internal data structure
  • after running the test, the configuration is notified about the result
  • after all tests are done, the configuration is notified about how many tests passed/failed/...

Almost all of the tests inside tests/html/* use either

  • the normal html configuration (pkg/unittest/lib/html_config.dart)
  • or the html individual configuration (pkg/unittest/lib/html_individual_config.dart)

The way the unittest framework surfaces the test results depends on the configuration that is being used. The html configurations print a nice looking table into the DOM once the test is done (with formatted stack traces ...). There are no uncaught exceptions on window.onerror.

But maybe I'm overlooking something?

@peter-ahe-google
Copy link
Contributor Author

@martin: Let's look at this together soon. It sounds to me like something isn't right with some of the configurations. The intent was, AFAIK, to always throw an exception if there was an error.

I can already tell that something is fishy by looking at the default implementation of Configuration.onDone. It must throw an exception if success is false. Also, someone added throwOnTestFailures which makes it optional n SimpleConfiguration.

@peter-ahe-google peter-ahe-google added Type-Defect area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. labels Jan 2, 2014
@kevmoo kevmoo added type-bug Incorrect behavior (everything from a crash to more subtle misbehavior) and removed priority-unassigned labels Feb 29, 2016
@matanlurey matanlurey added the closed-obsolete Closed as the reported issue is no longer relevant label Jun 19, 2018
@matanlurey
Copy link
Contributor

I'm guessing we no longer care about ContentShell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-infrastructure Use area-infrastructure for SDK infrastructure issues, like continuous integration bot changes. closed-obsolete Closed as the reported issue is no longer relevant type-bug Incorrect behavior (everything from a crash to more subtle misbehavior)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kevmoo @matanlurey @peter-ahe-google @mkustermann