Skip to content

Moving the error handling out of network loader#9942

Merged
bors-servo merged 4 commits intoservo:masterfrom
jdm:load_error
Apr 20, 2016
Merged

Moving the error handling out of network loader#9942
bors-servo merged 4 commits intoservo:masterfrom
jdm:load_error

Conversation

@jdm
Copy link
Copy Markdown
Member

@jdm jdm commented Mar 9, 2016

Rebase of #8851. Fixes #8678. Fixes #9944.

This change is Review on Reviewable

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Mar 9, 2016
@ghost ghost mentioned this pull request Mar 9, 2016
Closed
@jdm
Copy link
Copy Markdown
Member Author

jdm commented Mar 9, 2016

Note, currently writing tests for the security properties of chrome_loader.rs.

@jdm
Copy link
Copy Markdown
Member Author

jdm commented Mar 9, 2016

Tests written. I've reviewed all the commits by @Wafflespeanut but someone else should review my changes in the final commit.

@jdm jdm mentioned this pull request Mar 9, 2016
Closed
@KiChjang KiChjang closed this Mar 9, 2016
@KiChjang KiChjang reopened this Mar 9, 2016
@Ms2ger
Copy link
Copy Markdown
Contributor

Ms2ger commented Mar 18, 2016

LGTM

@jdm jdm assigned Ms2ger and unassigned glennw Mar 18, 2016
@jdm
Copy link
Copy Markdown
Member Author

jdm commented Mar 18, 2016

@bors-servo: r=jdm,Ms2ger

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Mar 18, 2016

📌 Commit 15bed8e has been approved by jdm,Ms2ger

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Mar 18, 2016
@jdm
Copy link
Copy Markdown
Member Author

jdm commented Mar 18, 2016

@bors-servo: r-
It's not clear from the appveyor log that the tests actually ran.

@highfive highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Mar 18, 2016
@Ms2ger
Copy link
Copy Markdown
Contributor

Ms2ger commented Mar 21, 2016

Running C:/projects/servo/target\debug\deps\gfx_tests-34a41c7816b6d727.exe
Command exited with code 127

@jdm
Copy link
Copy Markdown
Member Author

jdm commented Mar 21, 2016

Yeah. I'm guessing it's mach PATH shenanigans like all the times people have encountered that with ./mach run on Windows.

@jdm jdm force-pushed the load_error branch 3 times, most recently from 3c72ad7 to 39e991d Compare March 21, 2016 18:09
@Ms2ger
Copy link
Copy Markdown
Contributor

Ms2ger commented Mar 23, 2016

r? @larsbergstrom

@highfive highfive assigned larsbergstrom and unassigned Ms2ger Mar 23, 2016
@jdm jdm removed the S-needs-rebase There are merge conflict errors. label Apr 20, 2016
@jdm
Copy link
Copy Markdown
Member Author

jdm commented Apr 20, 2016

Waiting on the result of the appveyor test.

@jdm
Copy link
Copy Markdown
Member Author

jdm commented Apr 20, 2016

@bors-servo: r=ms2ger

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Apr 20, 2016

📌 Commit d888ed3 has been approved by ms2ger

@highfive highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. labels Apr 20, 2016
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Apr 20, 2016

⌛ Testing commit d888ed3 with merge f051028...

bors-servo pushed a commit that referenced this pull request Apr 20, 2016
Auto merge of #9942 - jdm:load_error, r=ms2ger
f051028 
Moving the error handling out of network loader

Rebase of #8851. Fixes #8678. Fixes #9944.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9942)
<!-- Reviewable:end -->
@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Apr 20, 2016

💔 Test failed - linux-rel

@highfive highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Apr 20, 2016
@KiChjang
Copy link
Copy Markdown
Contributor

KiChjang commented Apr 20, 2016 

Ran 4682 tests finished in 361.0 seconds.
  • 4680 ran as expected. 1363 tests skipped.
  • 1 tests failed unexpectedly
  • 1 tests had unexpected subtest results

Tests with unexpected results:
  ▶ Unexpected subtest result in /html/browsers/browsing-the-web/scroll-to-fragid/scroll-frag-percent-encoded.html:
  └ PASS [expected FAIL] Fragment Navigation: fragment id should be percent-decoded

  ▶ FAIL [expected PASS] /_mozilla/css/incremental_trailing_whitespace_a.html
  └   → /_mozilla/css/incremental_trailing_whitespace_a.html da54041086f2975f8e3c776d2283ad6609e6862a
/_mozilla/css/incremental_trailing_whitespace_ref.html f5d0147c8bddbdc772a7e5e86c7c9e433fcd486b
Testing da54041086f2975f8e3c776d2283ad6609e6862a == f5d0147c8bddbdc772a7e5e86c7c9e433fcd486b

@jdm
Copy link
Copy Markdown
Member Author

jdm commented Apr 20, 2016

@bors-servo: retry

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Apr 20, 2016

⚡ Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css are reusable. Rebuilding only linux-rel, mac-rel-wpt...

@bors-servo
Copy link
Copy Markdown
Contributor

bors-servo commented Apr 20, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt

@bors-servo bors-servo merged commit d888ed3 into servo:master Apr 20, 2016
@highfive highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Apr 20, 2016
This was referenced Apr 20, 2016
Closed
Merged
Merged
Merged
Merged
@jdm jdm mentioned this pull request May 24, 2016
Closed
@mrobinson mrobinson mentioned this pull request Jul 3, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@Ms2ger Ms2ger

Labels

S-tests-failed The changes caused existing tests to fail.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@jdm @Ms2ger @bors-servo @larsbergstrom @frewsxcv @KiChjang @glennw @highfive