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

Run style tests with all properties included #13386

Merged
merged 2 commits into from Sep 24, 2016

Conversation

Projects
None yet
4 participants
@Manishearth
Member

Manishearth commented Sep 23, 2016

This turns on all properties (including stylo-only ones) when the style unit tests are compiled. This lets us test the parsing of gecko-only properties.

These tests can't go in tests/stylo since this introduces dependencies on Gecko_* symbols. This method lets us test parsing and serialization of geckolib properties compiled in Servo mode (e.g. using Servo atoms, Servo URLs)

r? @emilio


This change is Reviewable

@highfive

This comment has been minimized.

Show comment
Hide comment
@highfive

highfive Sep 23, 2016

Heads up! This PR modifies the following files:

  • @bholley: components/style/Cargo.toml, components/style/properties/build.py, components/style/build.rs, components/style/properties/data.py
  • @wafflespeanut: python/servo/testing_commands.py

Heads up! This PR modifies the following files:

  • @bholley: components/style/Cargo.toml, components/style/properties/build.py, components/style/build.rs, components/style/properties/data.py
  • @wafflespeanut: python/servo/testing_commands.py
@highfive

This comment has been minimized.

Show comment
Hide comment
@highfive

highfive Sep 23, 2016

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!

warning Warning warning

  • These commits modify style code, but no tests are modified. Please consider adding a test!

@Manishearth Manishearth referenced this pull request Sep 23, 2016

Merged

Implement parsing for mask shorthand #13336

4 of 4 tasks complete
@emilio

r=me, ideally with a doc comment in the data file about the testing stuff and why we do it this way.

Thanks!

Show outdated Hide outdated components/style/build.rs
@@ -55,6 +55,7 @@ fn main() {
.arg(&script)
.arg(product)
.arg("style-crate")
.arg(if cfg!(feature = "testing") {"testing"} else {"regular"})

This comment has been minimized.

@emilio

emilio Sep 23, 2016

Member

I'm surprised tidy doesn't complain about the spacing around braces here

@emilio

emilio Sep 23, 2016

Member

I'm surprised tidy doesn't complain about the spacing around braces here

Show outdated Hide outdated components/style/properties/build.py
@@ -23,10 +23,12 @@ def main():
abort(usage)
product = sys.argv[1]
output = sys.argv[2]
testing = len(sys.argv) > 3 and sys.argv[3] == "testing"

This comment has been minimized.

@emilio

emilio Sep 23, 2016

Member

We're always passing this argument now, right? can you make this just sys.argv[3] == "testing"?

@emilio

emilio Sep 23, 2016

Member

We're always passing this argument now, right? can you make this just sys.argv[3] == "testing"?

This comment has been minimized.

@Manishearth

Manishearth Sep 23, 2016

Member

We're not always passing this argument; style_tests itself contains a test that calls this without the third argument. I could add it, but it seems better to ignore.

@Manishearth

Manishearth Sep 23, 2016

Member

We're not always passing this argument; style_tests itself contains a test that calls this without the third argument. I could add it, but it seems better to ignore.

This comment has been minimized.

@Manishearth

Manishearth Sep 23, 2016

Member

Eh, I'll add it

@Manishearth

Manishearth Sep 23, 2016

Member

Eh, I'll add it

@Manishearth

This comment has been minimized.

Show comment
Hide comment
Member

Manishearth commented Sep 23, 2016

@bors-servo r=emilio

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Sep 23, 2016

Contributor

📌 Commit 065032d has been approved by emilio

Contributor

bors-servo commented Sep 23, 2016

📌 Commit 065032d has been approved by emilio

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Sep 23, 2016

Contributor

⌛️ Testing commit 065032d with merge 4affcf3...

Contributor

bors-servo commented Sep 23, 2016

⌛️ Testing commit 065032d with merge 4affcf3...

bors-servo added a commit that referenced this pull request Sep 23, 2016

Auto merge of #13386 - Manishearth:style-testing, r=emilio
Run style tests with all properties included

This turns on all properties (including stylo-only ones) when the style unit tests are compiled. This lets us test the parsing of gecko-only properties.

These tests can't go in tests/stylo since this introduces dependencies on `Gecko_*` symbols. This method lets us test parsing and serialization of geckolib properties compiled in Servo mode (e.g. using Servo atoms, Servo URLs)

r? @emilio

<!-- 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/13386)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Sep 23, 2016

Contributor

💔 Test failed - linux-rel

Contributor

bors-servo commented Sep 23, 2016

💔 Test failed - linux-rel

@highfive

This comment has been minimized.

Show comment
Hide comment
@highfive

highfive Sep 23, 2016

  ▶ Unexpected subtest result in /_mozilla/mozilla/promise.html:
  │ FAIL [expected PASS] Native promise from async callback can be resolved
  │   → assert_true: expected true got false
  │ 
  └ @http://web-platform.test:8000/_mozilla/mozilla/promise.html:58:11
  ▶ Unexpected subtest result in /_mozilla/mozilla/promise.html:
  │ FAIL [expected PASS] Native promise from async callback can be resolved
  │   → assert_true: expected true got false
  │ 
  └ @http://web-platform.test:8000/_mozilla/mozilla/promise.html:58:11
@Manishearth

This comment has been minimized.

Show comment
Hide comment
@Manishearth

Manishearth Sep 23, 2016

Member

@bors-servo retry

Looks like the bug mentioned in IRC earlier, but can't be related with this. Let's see if it's an intermittent.

Member

Manishearth commented Sep 23, 2016

@bors-servo retry

Looks like the bug mentioned in IRC earlier, but can't be related with this. Let's see if it's an intermittent.

Show outdated Hide outdated components/style/properties/build.py
@@ -18,15 +18,17 @@
def main():
usage = "Usage: %s [ servo | gecko ] [ style-crate | html ]" % sys.argv[0]
usage = "Usage: %s [ servo | gecko ] [ style-crate | html ] [ testing | regular ]" % sys.argv[0]
if len(sys.argv) < 3:

This comment has been minimized.

@emilio

emilio Sep 23, 2016

Member

nit: This needs to become < 4, right?

@emilio

emilio Sep 23, 2016

Member

nit: This needs to become < 4, right?

@Manishearth

This comment has been minimized.

Show comment
Hide comment
@Manishearth

Manishearth Sep 23, 2016

Member

@bors-servo r=emilio

done

Member

Manishearth commented Sep 23, 2016

@bors-servo r=emilio

done

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Sep 23, 2016

Contributor

📌 Commit c5a7bb2 has been approved by emilio

Contributor

bors-servo commented Sep 23, 2016

📌 Commit c5a7bb2 has been approved by emilio

@bors-servo

This comment has been minimized.

Show comment
Hide comment
@bors-servo

bors-servo Sep 24, 2016

Contributor

⌛️ Testing commit c5a7bb2 with merge 28bce69...

Contributor

bors-servo commented Sep 24, 2016

⌛️ Testing commit c5a7bb2 with merge 28bce69...

bors-servo added a commit that referenced this pull request Sep 24, 2016

Auto merge of #13386 - Manishearth:style-testing, r=emilio
Run style tests with all properties included

This turns on all properties (including stylo-only ones) when the style unit tests are compiled. This lets us test the parsing of gecko-only properties.

These tests can't go in tests/stylo since this introduces dependencies on `Gecko_*` symbols. This method lets us test parsing and serialization of geckolib properties compiled in Servo mode (e.g. using Servo atoms, Servo URLs)

r? @emilio

<!-- 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/13386)
<!-- Reviewable:end -->
@bors-servo

This comment has been minimized.

Show comment
Hide comment
Contributor

bors-servo commented Sep 24, 2016

@bors-servo bors-servo merged commit c5a7bb2 into servo:master Sep 24, 2016

2 of 3 checks passed

continuous-integration/appveyor/pr AppVeyor build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment