-
Notifications
You must be signed in to change notification settings - Fork 115
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
Sync generated test suite with Yoga #285
Comments
Yep: yoga team, I'd love to help you out like this if you wanna steal our tests (and approaches to implementing algorithms) :D |
I really appreciate this 🙂. It also helps to catalog some of the things Yoga doesn't currently support. I'm not sure what the right mechanism is for pushing/pulling, but we could maybe start by keeping a flat set of synced fixtures. Then specific engines would maintain their own list of known failures as part of the test generation or test runner. Apart from Yoga missing Taffy capabilities, I think we need that anyway:
|
Yeah, I'm wondering if at least for taffy we should have a Chrome compatibility mode controlled by a feature. And then attempt to use the "most natural" behavior in cases where we think Chrome has bugs. |
Regarding syncing Taffy/Yoga's existing test suites:
I agree. This definitely seems like the most sensible way to start. We'll probably want to harmonise the format used to store the fixtures: @alice-i-cecile I personally feel like Yoga's format which stores multiple related fixtures in a single file separated by two newlines is nicer / more manageable than Taffy's format where each test is in it's own file with it's copy of the test template / boilerplate. Although the one thing I do really like about Taffy's approach is that it means the HTML file can be opened and played with directly in a web browser without any setup. Perhaps we could create a small CLI tool which:
to retain this capability while switching to Yoga's on-disk storage format. Taffy's approach does also make it easier to enumerate tests as one can do so just by listing files without parsing them. But parsing the Yoga format isn't all that difficult either. It may also be possible to go for an in-between approach of "single test per file, but no surrounding boilerplate". If we do go for "single test per file" I'd definitely like to introduce sub-directories though. The current flat list of fixtures is quite unwieldy! We may also want to harmonise the test generation scripts: This isn't really necessary – we could also keep these separate – but perhaps we could share maintenance by merging implementations. My guess is this would work out as less work overall if communication between projects is good, and would become problematic if it's not. Taffy currently implements test generation mostly in Rust, with only a small JavaScript script that inside Chrome measures nodes and returns computed styles but doesn't actually do test generation. Yoga currently has a thin Ruby wrapper (64 LOC) which runs chromedriver, and the bulk of the test generation is done using JavaScript which runs within Chrome. This JS script has "plugins" for each of Yoga's language bindings so that the tests can be generated for them too (yoga-rs also uses Yoga's script and has a compatible "plugin" for it's Rust bindings). If we were to harmonise this, my suggestion would be to replace Yoga's Ruby wrapper with a Rust wrapper (on the basis that setting up a Rust toolchain and installing dependencies is much simpler than managing Ruby versions and it's tendency to install gems globally), but to port Taffy's actual code generation to Yoga's JS approach (we'd need to extend this to deal with Grid and text-layout).
Yes, I think this makes a lot of sense (my instinct is to make it part of test generation, but I guess it probably doesn't matter too much). Regarding intentional divergence from Chrome:
I wonder if it would be worth defining some kind of intermediate representation for "expected measurements" (perhaps a simple JSON tree with Regarding importing tests from WPT:I definitely think this makes sense, but I think this is going to be fairly large task. Some notes from a quick look at the available tests:
Given this, I would propose the following:
I'm probably going to focus on getting an MVP CSS Grid implementation merged into Taffy in the immediate future, and circle back around to this later. |
Mild preference for this format :) I think it's clearer and easy to parse and diff.
Agreed, this seems like an excellent compromise. Totally fine to change the test format however you see fit.
Sensible, but can be deferred.
On board with this! @mockersf, do you have strong feelings or good ideas here? You've helped us with the automated tests before and I trust your instincts on this. |
I've created #320 for importing tests. It includes a small script for diffing the names which is much better than the manual process I used first time around as it's automated and patches up the names of some tests that are in fact the same. I've updated the diff output in the top post in the following way:
I plan to proceed by:
We can then go through and fix the failing tests at our leisure. |
Excellent work: I'm excited to share these. |
Ok, #320 now includes all of Yoga's test fixtures (including those from facebook/yoga#1194) and is ready for review/merging. I've replaced the raw diff in the description with two separate lists:
|
CC: @NickGerleman
What problem does this solve or what need does it fill?
The more tests we have the better! Our generated test system is derived from Yoga's and the format is mostly compatible. And Yoga has a bunch of tests in their system that we don't have (some of these may not be applicable, but many of them will be). We have also added tests that Yoga doesn't have, and we should be good OSS citizens and make them available for Yoga.
What solution would you like?
Tests Taffy is missing
As of #320, Taffy has now imported all of Yoga's tests, however the following tests are disabled (by prefixing the filename with an "x") because they are failing:
Tests Yoga is missing
This list excludes CSS Grid tests as out of scope:
The text was updated successfully, but these errors were encountered: