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.

Sign up for GitHub

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

try/poi/serialize test is broken #25125

Closed
whesse opened this issue Dec 7, 2015 · 3 comments
Closed

try/poi/serialize test is broken #25125

whesse opened this issue Dec 7, 2015 · 3 comments
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). P2 A bug or feature request we're likely to work on

Comments

@whesse
Copy link
Contributor

whesse commented Dec 7, 2015

@fsc8000 @peter-ahe-google

The --no-opt change broke serialization in try/poi. The test is in tests/try/poi/serialize_test.dart, and the status is in tests/try/try.status.

The failure can be reproduced with tools/test.py -mrelease -rvm -cnone try/poi/serialize

The expected string has line breaks, and the produced one doesn't, or vice versa.
The commits where the test started failing on the buildbot are:
Change #43285

Changed by fschneider@google.com
Changed at Fri 04 Dec 2015 04:08:14
Repository https://chromium.googlesource.com/external/github.com/dart-lang/sdk.git
Project sdk-master
Branch master
Revision 2b7ff6d
Comments

Fix test by removing the --noopt flag from the test file.
Not all platforms (ia32) support --noopt, and test.py --noopt will run the test
with this option anyway.

TBR=rmacnak@google.com

BUG=

Review URL: https://codereview.chromium.org/1497213002 .
Changed files

tests/language/vm/precompiled_static_initializer_test.dart
@whesse whesse added Priority-Medium area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). labels Dec 7, 2015
@fsc8000
Copy link
Contributor

fsc8000 commented Dec 7, 2015

I don't see how my change affected that test. Are you sure that you bisected correctly?

whesse added a commit that referenced this issue Dec 7, 2015
@whesse
Update status for two failing tests
c49e39f 
BUG=#24571
BUG=#25125

TBR=fschneider@google.com

Review URL: https://codereview.chromium.org/1504003002 .
@fsc8000 fsc8000 removed their assignment Dec 7, 2015
@fsc8000
Copy link
Contributor

fsc8000 commented Dec 7, 2015

bisected to fc868df @sigurdm

@sigurdm
Copy link
Contributor

sigurdm commented Dec 7, 2015

Sorry for breaking this.
Proposed fix: https://codereview.chromium.org/1505743003/

On Mon, Dec 7, 2015 at 3:39 PM, Florian Schneider notifications@github.com
wrote:

bisected to fc868df
fc868df
@sigurdm https://github.com/sigurdm

—
Reply to this email directly or view it on GitHub
#25125 (comment).

sigurdm added a commit that referenced this issue Dec 7, 2015
@sigurdm
Change the order the classes are expected to be found by serialize_test
50b17cf 
This solves test failure triggered by:
https://codereview.chromium.org/1498963004/
Fixes issue #25125

R=fschneider@google.com

Review URL: https://codereview.chromium.org/1505743003 .
@sigurdm sigurdm closed this as completed Dec 7, 2015
@kevmoo kevmoo added P2 A bug or feature request we're likely to work on and removed Priority-Medium labels Mar 1, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). P2 A bug or feature request we're likely to work on
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kevmoo @fsc8000 @whesse @sigurdm