Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdd tests for background shorthand parsing #14271
Conversation
highfive
commented
Nov 18, 2016
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @larsbergstrom (or someone else) soon. |
|
r? @canaltinova @bors-servo delegate=canaltinova |
|
|
|
Code looks good. Thanks for doing this! |
|
|
| use style::properties::longhands::{background_origin, background_position, background_repeat, background_size}; | ||
| use style::properties::shorthands::background; | ||
| use style::stylesheets::Origin; | ||
| use url::Url; |
This comment has been minimized.
This comment has been minimized.
Manishearth
Nov 18, 2016
Member
This will have to be servo_url::ServoUrl (and youll have to replace occurrences of Url below with ServoUrl). URL handling for style was changed since you made this pull request, which is why Travis CI is failing.
Please rebase over master and make this change (let me know if you need help!)
|
So, the tests will fail anyway. @bors-servo r- |
fdd8bf6
to
9f49da9
|
Ah, thanks for the pointer @Manishearth. Fixed and pushed |
|
Oh right, I missed the new url. Thanks for pointing it @Manishearth! |
|
|
Add tests for background shorthand parsing <!-- Please describe your changes on the following line: --> This PR adds tests for parsing the background shorthand. Copies & adapts the tests from mask.rs (minus the mode test), and adds a test for comma separated declarations. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #13548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [X] These changes do not require tests because they are tests <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- 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/14271) <!-- Reviewable:end -->
|
|
highfive
commented
Nov 19, 2016
|
|
@bors-servo retry |
Add tests for background shorthand parsing <!-- Please describe your changes on the following line: --> This PR adds tests for parsing the background shorthand. Copies & adapts the tests from mask.rs (minus the mode test), and adds a test for comma separated declarations. --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #13548 (github issue number if applicable). <!-- Either: --> - [ ] There are tests for these changes OR - [X] These changes do not require tests because they are tests <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- 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/14271) <!-- Reviewable:end -->
|
|
linclark commentedNov 18, 2016
•
edited by larsbergstrom
This PR adds tests for parsing the background shorthand.
Copies & adapts the tests from mask.rs (minus the mode test), and adds a test for comma separated declarations.
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is