Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Flaky test results when using DumpRenderTree #7536

Closed
mkustermann opened this issue Dec 20, 2012 · 10 comments
Closed

Flaky test results when using DumpRenderTree #7536

mkustermann opened this issue Dec 20, 2012 · 10 comments
Assignees
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). closed-duplicate Closed in favor of an existing report

Comments

@mkustermann
Copy link
Member

Sometimes DumpRenderTree writes the following to stdout but still exits with an exit code of 1.
"""
Content-Type: text/plain
PASS
1 PASS Expectation: constructorTest.
2 PASS Expectation: createBuffer.
3 PASS Expectation: audioRenames.
All 3 tests passed

EOF

"""

Thus, although all the tests passed the exit code is 1 and the test is therefore reported as failed.

Here is a log which contains such a failure:
  http://chromegw.corp.google.com/i/client.dart/builders/dart2js-chrome-linux/builds/1934/steps/dart2js%20tests/logs/stdio
In this log the "html/audiocontext_test" test failed although all tests passed.

I looked into this issue with the following results:

  1. Reproduce it locally

To reproduce it locally, I run the test once with test.py so the html file gets created. After that I run DumpRenderTree directly with the generated html file.

Since this bug doesn't appear each time I run it in a loop. It seems that in 5%-20% of the cases DRT exits with exit-code 1.

  1. Reason for Exit code != 0

I looked into why we get an exit code != 0 even if all tests pass. After some tests, I found out that each time the test exits with exit-code 1, a SIGSEGV is delivered to the DumpRenderTree.
To see that, I built DumpRenderTree myself and installed a custom SIGSEGV signal handler (in "third_party/WebKit/Tools/DumpRenderTree/chromium/DumpRenderTree.cpp").

Then I inserted some code to the signal handler which prints the backtrace (unfortunatly the backtrace doesn't contain function symbols). But after some further debugging, I found out that (at least in the cases I saw) there is a null pointer dereference in:
  file: third_party/WebKit/Source/WebCore/platform/audio/chromium/AudioBusChromium.cpp
  function: AudioBus::loadPlatformResource

It seems like "WebKit::Platform::current()" returns NULL, and since that is dereferenced, the SIGSEGV gets delivered.

@anton: Could you take a look into this?

@mkustermann
Copy link
Member Author

One more note: I don't think that this is an "html/audiocontext_test" only issue. The failure of "html/websql_test" in
  http://chromegw.corp.google.com/i/client.dart/builders/dartium-lucid64-full/builds/3802/steps/drt_core_checked_tests/logs/stdio
looks very similar, for example.

@mkustermann
Copy link
Member Author

Removed Priority-Medium label.
Added Priority-Critical label.

@DartBot
Copy link

DartBot commented Dec 20, 2012

This comment was originally written by antonm@google.com


Added Started label.

@DartBot
Copy link

DartBot commented Dec 21, 2012

This comment was originally written by antonm@google.com


Alas, I am not sure what we should do with that.

Ideally we should fix underlying issue, but I am not versed in the stuff which is to blame and this solution most probably do not scale: even if I can fix all the issues we meet (and there are apparently at least two of them), time to debug, review and upstream it might be too big.

Hence another proposal: we already have a mechanism for potential flaks in test framework, cannot we extend it to cover those cases as well: if output says we're okay, but DRT crashes, run test another time.

Or even more radical: ignore exit code and only trust stdout.

That should scale ways better even though not the best option in the world.

Thoughts, gentlemen (and ladies, of course, if they read this).

@mkustermann
Copy link
Member Author

I agree with anton -- we should ignore the exit code of DRT and only trust stdout.

The thing is, that everytime we roll forward to a new chrome/webkit version we risk using a buggy DumpRenderTree version.

@ricowind
Copy link
Contributor

Lets do that. If exitcodes are flaky we may as well just use stdout. Are there chromium bugs for this? Running it twice is a pretty bad solution IMHO (I know even WebKit test runner does this but anyway)

@DartBot
Copy link

DartBot commented Dec 21, 2012

This comment was originally written by antonm@google.com


I found no immediate bug, but haven't spent too much time looking for it.

If consensus is to trust stdout only, I am more than happy. And just closing this bug.

Martin, thanks a lot for digging this out---that should have been non trivial.


Added WontFix label.

@DartBot
Copy link

DartBot commented Dec 21, 2012

This comment was originally written by @aam


There is another process exit code problem that was hit when running dart2js test suite on Windows: dartbug.com/6062

@ricowind
Copy link
Contributor

ricowind commented Jan 3, 2013

We are going to keep an eye on this moving forward using log data from the buildbot with the new debug information. If this keeps happening we are rather positive that we have a real issue here in either the bindings or the dart vm - I don't expect the chromium people to allow this kind of behavior by default in DRT.

Reoppening the bug with a lower priority, assigning over to myself for now to track this


Set owner to @ricowind.
Removed Priority-Critical label.
Added Priority-Medium, Waiting labels.

@ricowind
Copy link
Contributor

Added Duplicate label.
Marked as being merged into #15139.

@mkustermann mkustermann added Type-Defect area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). closed-duplicate Closed in favor of an existing report labels Nov 25, 2013
This issue was closed.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). closed-duplicate Closed in favor of an existing report
Projects
None yet
Development

No branches or pull requests

3 participants