Fix unit tests and run them on CI - #436
Conversation
| #[test] | ||
| fn parse_valid_mapping_list() { | ||
| let mut mappings = parse_mapping_list("foo: bar, totally-invalid-mapping,,"); | ||
| let mut mappings = parse_mapping_list("foo: bar, totally-invalid-mapping:,,"); |
There was a problem hiding this comment.
This test was failing: the second call to mappings.next() expected None but instead got Some(("totally-invalid-mapping", "totally-invalid-mapping")) which did look valid per:
https://drafts.csswg.org/css-shadow-1/#parsing-mapping
- If the end of the input has been reached, return the tuple (first token, first token)
This modified test case goes through here instead:
- If second token is empty then return error.
| - name: Run Tests | ||
| run: cargo build --features servo | ||
| run: cargo test --features servo |
There was a problem hiding this comment.
Good thing these steps were already named "Run Tests" :)
Signed-off-by: Simon Sapin <simon@igalia.com>
| run: cargo build --features servo | ||
| run: cargo test --features servo |
There was a problem hiding this comment.
Can we make these separately build and then test. Test doesn't always catch everything that build does.
There was a problem hiding this comment.
I don’t believe that’s true unless in specific configurations not used in the repo (the library crate is compiled as a dependency of doctests before rustdoc finds out whether there are any) but I don’t think it hurts either, so done.
I’ve also added --workspace to both commands, and removed --features servo since it’s the default.
There was a problem hiding this comment.
I think I've had issues with feature flags before. And of course #[cfg(test)] can affect things. But I'm happy to go with this this if you think it's fine. We can always change it later if we run into issues.
|
Note: Stylo also has integration tests in:
I believe these are two copies of the same tests, which are probably no longer in sync with each other. Might be worth reconciling those copies. If we could get the tests fixed/running then we could upstream our fixes to the Firefox copy, start syncing them into this repo as part of our regular syncs, and delete the copy in the Servo repo. |
Signed-off-by: Simon Sapin <simon@igalia.com>
And in particular it would be good to make sure they do run in Firefox, so that when refactorings happen upstream, I don't have to adapt the tests |
|
(Thanks for the approvals but I don’t have access to add to the merge queue for this repo) |
4 participants
No description provided.