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

TypedMessageTest on dartium-lucid64-inc is flaky #2255

Closed
peter-ahe-google opened this issue Mar 22, 2012 · 16 comments
Closed

TypedMessageTest on dartium-lucid64-inc is flaky #2255

peter-ahe-google opened this issue Mar 22, 2012 · 16 comments
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). library-isolate

Comments

@peter-ahe-google
Copy link
Contributor

The log contains "PASS

EOF" so the test should be passing, I assume.

It looks like there is more output after #EOF. Perhaps this is a race condition in testing of isolates. So my guess is that either the test is flaky, or the framework doesn't handle isolates correctly.

FAILED: dartium release_ia32 TypedMessageTest
Expected: pass
Actual: fail

stdout:
Content-Type: text/plain
[print]: Starting log server.
PASS

EOF

[print]: Log [0, 1, 2, 3, 4]
[print]: Stopping log server.

Command line: python /mnt/data/b/build/slave/dartium-lucid64-inc/build/src/dart/tools/testing/drt-trampoline.py /mnt/data/b/build/slave/dartium-lucid64-inc/build/src/out/Release/DumpRenderTree --no-timeout --dart-flags=--ignore-unrecognized-flags --enable_type_checks --enable_asserts /mnt/data/b/build/slave/dartium-lucid64-inc/build/src/out/Release_ia32/generated_tests/dartium/tests_language_src_TypedMessageTest-/test.html

@sigmundch
Copy link
Member

Currently we check that a test passes if PASS is the first line right after 'Content-Type:text/plain'. In this example it seems that the log message ([print]: ...) happened while the page was being printed.

I think we should simply avoid printing anything in our tests.

@peter-ahe-google
Copy link
Contributor Author

Sorry, Siggi, I couldn't disagree more :-)

It is important that our tests can test every thing a user throws at it, and it is important that we can debug test failures.

We must be allowed to use print, just as we must be allowed to use scripts without library names.

Any feature we disallow in tests should be considered to not be implemented correctly, because we aren't testing it.

@sigmundch
Copy link
Member

Ok, let's make the test framework more robust to avoid these issues. We basically need to change unittest and test.dart so we can ignore any printed content (except for the "PASS" or "FAIL" indicator).

Since this is not a test for 'print', I'm thinking that we should anyways remove the prints on this particular test :-)

@a-siva
Copy link
Contributor

a-siva commented Mar 26, 2012

The test should fail consistently on all architectures not just
dartium_lucid64-inc if the problem is just the extra 'print'
statements. Is that true. does it always fail consistently on all
runs?

@sigmundch
Copy link
Member

The problem in this case is that the print was interleaved with the output of the page.

We ask DumpRenderTree to print the page in a text format so we can extract the status of the test.

Content-Type:text/plain
PASS
#EOF

As long as the prints occur before or after, we are good. In chrome, all the prints occur before the text output, for some reason in dartium the prints are flushed later and show up interleaved.

@peter-ahe-google
Copy link
Contributor Author

Although this isn't a test of print, the print statements may be instrumental in debugging test failures.

Here is my take:

* the test framework must support any language feature or library feature we throw at it (except for deleting everything, etc)

* the test framework should not suppress output. You can spend a lot of time debugging why there is not output :-)

@efortuna
Copy link
Contributor

+whesse, for test.dart change


cc @whesse.

@DartBot
Copy link

DartBot commented Mar 27, 2012

This comment was originally written by whe...@gmail.com


We could change the browser tests so they accept stuff between text/plain and PASS and #EOF, but then what if we get output between PA and SS?
I don't see why DumpRenderTree is outputting the page while Dart content is still running - shouldn't the outputting of the page be atomic? Maybe DumpRenderTree could put all of its page output together, and print it to stdout atomically.

I think making test.dart more tolerant in what it accepts might fix the current problems, but there is a real problem in the way Dartium DRT is outputting the page.

@sigmundch
Copy link
Member

Added Isolates label.

@whesse
Copy link
Contributor

whesse commented Jun 14, 2012

Added this to the Later milestone.

@sigmundch
Copy link
Member

Added Library-Isolates label.

@sigmundch
Copy link
Member

Removed Isolates label.

@dgrove
Copy link
Contributor

dgrove commented Jan 11, 2013

Added Library-Isolate label.

@dgrove
Copy link
Contributor

dgrove commented Jan 11, 2013

Removed Library-Isolates label.

@ricowind
Copy link
Contributor

We already ignore stuff interleaved, closing this

@ricowind
Copy link
Contributor

Added Fixed label.

@peter-ahe-google peter-ahe-google added Type-Defect area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). library-isolate labels Nov 15, 2013
@peter-ahe-google peter-ahe-google added this to the Later milestone Nov 15, 2013
copybara-service bot pushed a commit that referenced this issue Oct 24, 2023
Revisions updated by `dart tools/rev_sdk_deps.dart`.

dartdoc (https://github.com/dart-lang/dartdoc/compare/f7e9b17..53da3e1):
  53da3e1d  2023-10-23  dependabot[bot]  Bump actions/checkout from 4.1.0 to 4.1.1 (#3544)
  62d5469d  2023-10-23  Sam Rawlins  Support extension types in sidebars and categories (#3537)
  69abd0f0  2023-10-22  Sam Rawlins  Refactor file-gathering logic (#3539)
  e9c61d1d  2023-10-22  Sam Rawlins  Convert some late final fields to getters in Accessor, Annotation, Constructor (#3534)
  adcdc8b7  2023-10-20  Parker Lougheed  Update package:lints to v3 (#3542)
  41f02622  2023-10-19  Sam Rawlins  Greatly simplify ModelCommentReference (#3541)
  50e4b679  2023-10-19  Sam Rawlins  Remove use of NodeLocator2 (#3538)
  b2de813c  2023-10-19  Sam Rawlins  Remove support for deprecated leading new in comment references (#3529)
  b350c688  2023-10-19  Sam Rawlins  Bump to 7.0.1 (#3540)
  5256e2fb  2023-10-19  Sam Rawlins  Remove unused warnings (#3533)

lints (https://github.com/dart-lang/lints/compare/975c687..2cf8403):
  2cf8403  2023-10-23  Devon Carew  add no_wildcard_variable_uses; rev to a new major version (#165)
  7b0f556  2023-10-23  Parker Lougheed  Remove mention of no_wildcard_variable_uses from changelog (#164)

matcher (https://github.com/dart-lang/matcher/compare/356e5f6..7512f80):
  7512f80  2023-10-23  Michael Goderbauer  Specify language in neverCalled docs for nicer formatting in docs (#230)

protobuf (https://github.com/dart-lang/protobuf/compare/050c162..3528fad):
  3528fad  2023-10-24  Ömer Sinan Ağacan  Use `setRange` when copying output chunks to the final buffer in `CodedBufferWriter` (#887)

tools (https://github.com/dart-lang/tools/compare/15cc9c7..da6bb18):
  da6bb18  2023-10-24  Elias Yishak  Enum + event constructors added for doctor events (#178)
  e3dd149  2023-10-24  Elias Yishak  Use futures list internally to manage send events (#184)

webdev (https://github.com/dart-lang/webdev/compare/1bd434b..6e324af):
  6e324afb  2023-10-24  Ben Konyi  Add dependency on `package:vm_service_interface` (#2262)
  8429a79f  2023-10-20  Elliott Brooks  Only notify chatroom when daily stable testing fails (#2259)
  3463d169  2023-10-19  Elliott Brooks  Remove Chrome 115 extension error (#2258)
  fdebc06e  2023-10-19  Elliott Brooks  Reset Webdev to 3.2.0-wip after release (#2260)
  9cffb896  2023-10-19  Elliott Brooks  Reset DWDS to 22.1.0-wip after release (#2256)
  07c70c6c  2023-10-19  Elliott Brooks  Prepare Webdev for release to 3.1.0 (#2255)

Change-Id: Ieae3aadcc804a270867d7935b702987cf1f6d51c
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/332060
Reviewed-by: Elias Yishak <eliasyishak@google.com>
Commit-Queue: Devon Carew <devoncarew@google.com>
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). library-isolate
Projects
None yet
Development

No branches or pull requests

8 participants