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

Fix all AudioBuffer WPTs #21602

Merged
merged 10 commits into from Sep 19, 2018

Conversation

Projects
6 participants
@ferjm
Member

ferjm commented Sep 4, 2018

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • There are tests for these changes

This change is Reviewable

@highfive

This comment has been minimized.

highfive commented Sep 4, 2018

Heads up! This PR modifies the following files:

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 4, 2018

@bors-servo try=wpt

bors-servo added a commit that referenced this pull request Sep 4, 2018

Auto merge of #21602 - ferjm:audiobuffer.crash, r=<try>
Fix AudioBuffer crash

- [X] `./mach build -d` does not report any errors
- [ ] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21602)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 4, 2018

⌛️ Trying commit 54a7d4c with merge 94601ea...

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 4, 2018

@highfive highfive assigned Manishearth and unassigned emilio Sep 4, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 4, 2018

💔 Test failed - linux-rel-css

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 4, 2018

{"status": "CRASH", "group": "default", "message": null, "stack": null, "subtest": null, "test": "/webaudio/the-audio-api/the-audiobuffer-interface/ctor-audiobuffer.html", "line": 120970, "action": "test_result", "expected": "TIMEOUT"}
{"status": "CRASH", "group": "default", "message": null, "stack": null, "subtest": null, "test": "/webaudio/the-audio-api/the-audiobuffer-interface/audiobuffer-copy-channel.html", "line": 206207, "action": "test_result", "expected": "TIMEOUT"

Ugh

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 4, 2018

If I run these two tests independently, I get the crashes. If I run them together via ./mach test-wpt tests/wpt/web-platform-tests/webaudio/the-audio-api/the-audiobuffer-interface, I get timeouts.

@ferjm ferjm force-pushed the ferjm:audiobuffer.crash branch from 54a7d4c to a03de5d Sep 6, 2018

@highfive highfive removed the S-tests-failed label Sep 6, 2018

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 6, 2018

If I run these two tests independently, I get the crashes. If I run them together via ./mach test-wpt tests/wpt/web-platform-tests/webaudio/the-audio-api/the-audiobuffer-interface, I get timeouts.

This is fun. I am getting different behaviors on OSX and Linux.

On OSX I get what I described above (timeouts when running all tests together). On Linux, what the try run reported (crashes).

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 6, 2018

@bors-servo try=wpt

1 similar comment
@jdm

This comment has been minimized.

Member

jdm commented Sep 6, 2018

@bors-servo try=wpt

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 6, 2018

⌛️ Trying commit a03de5d with merge 743a15b...

bors-servo added a commit that referenced this pull request Sep 6, 2018

Auto merge of #21602 - ferjm:audiobuffer.crash, r=<try>
Fix AudioBuffer crash

- [X] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21602)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 6, 2018

💔 Test failed - linux-rel-wpt

@ferjm ferjm force-pushed the ferjm:audiobuffer.crash branch from a03de5d to 0e0e5a7 Sep 10, 2018

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 10, 2018

@bors-servo try=wpt

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 10, 2018

⌛️ Trying commit 0e0e5a7 with merge 826b627...

bors-servo added a commit that referenced this pull request Sep 10, 2018

Auto merge of #21602 - ferjm:audiobuffer.crash, r=<try>
Fix AudioBuffer crash

- [X] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21602)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 10, 2018

💔 Test failed - linux-rel-css

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 10, 2018

Regarding #21602 (comment), I am trying to fix all these crashes, so we don't introduce intermittent TIMEOUT vs CRASH failures.

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 18, 2018

@bors-servo try=wpt

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 18, 2018

⌛️ Trying commit c91d697 with merge 54aa075...

bors-servo added a commit that referenced this pull request Sep 18, 2018

Auto merge of #21602 - ferjm:audiobuffer.crash, r=<try>
Fix all AudioBuffer WPTs

- [X] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21602)
<!-- Reviewable:end -->

@ferjm ferjm removed the S-needs-rebase label Sep 18, 2018

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 18, 2018

☀️ Test successful - linux-rel-css, linux-rel-wpt
State: approved= try=True

@ferjm ferjm force-pushed the ferjm:audiobuffer.crash branch from c91d697 to 4a927b5 Sep 19, 2018

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 19, 2018

All AudioBuffer tests are passing now. I'll look at #21322 next. r? @Manishearth

@Manishearth

r=me

should there be test expectation updates here?

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 19, 2018

should there be test expectation updates here?

Oh, I updated the expectation, but it seems that I discarded the commit while force pushing (that's one of the issues of working with two different machines...).

@ferjm

This comment has been minimized.

Member

ferjm commented Sep 19, 2018

@bors-servo r=Manishearth

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 19, 2018

📌 Commit 9b35633 has been approved by Manishearth

@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 19, 2018

⌛️ Testing commit 9b35633 with merge 0866dab...

bors-servo added a commit that referenced this pull request Sep 19, 2018

Auto merge of #21602 - ferjm:audiobuffer.crash, r=Manishearth
Fix all AudioBuffer WPTs

- [X] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [X] There are tests for these changes

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/21602)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Contributor

bors-servo commented Sep 19, 2018

@bors-servo bors-servo merged commit 9b35633 into servo:master Sep 19, 2018

1 of 4 checks passed

Taskcluster (pull_request) TaskGroup: failure
Details
continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
homu Test successful
Details

@ferjm ferjm deleted the ferjm:audiobuffer.crash branch Sep 19, 2018

@ferjm ferjm added this to Done in WebAudio Nov 29, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment