stylo: Run other stylo tests in rusttests tasks

NEW
Unassigned

Status

defect
P3
normal
2 years ago
a year ago

People

(Reporter: xidorn, Unassigned)

Tracking

(Blocks 1 bug)

Trunk
Dependency tree / graph

Firefox Tracking Flags

(firefox-esr52 unaffected, firefox56 unaffected, firefox57 wontfix, firefox58 affected)

Details

(Reporter)

Description

2 years ago
It seems that bug 1373878 only enabled stylo struct layout test. I think it also makes sense to run function signature test to ensure that the generated functions are synced with those defined in glue.rs.

It may make sense to also run the size_of tests since there can be Rust structs which contain C++ data struct inside, and changes to C++ code can bloat the corresponding things unexpectedly.

These tests are probably lower priority, but should help avoid certain kind of regressions. Also I imagine that adding those tests shouldn't add too much burden to the existing test task.
(Reporter)

Comment 1

2 years ago
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #0)
> It seems that bug 1373878 only enabled stylo struct layout test. I think it
> also makes sense to run function signature test to ensure that the generated
> functions are synced with those defined in glue.rs.

I think I misstated it. The functions are not generated, but written manually in ServoBindingList.h.

So basically we are maintaining duplicate declarations of Servo_ binding functions, which is problematic. People may forget to update one of them, or to add things in ServoBindings.toml for new type mapping.

This can cause regression, but it is more likely to cause developer confusion because no error would be shown until someone tries to update Servo's in-tree binding files.
(Reporter)

Comment 2

2 years ago
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #1)
> So basically we are maintaining duplicate declarations of Servo_ binding
> functions, which is problematic. People may forget to update one of them, or
> to add things in ServoBindings.toml for new type mapping.

Bug 1406811 is an example of this.
Priority: -- → P3
Summary: Run other stylo tests in rusttests tasks → stylo: Run other stylo tests in rusttests tasks

Updated

a year ago
Product: Core → Firefox Build System
See Also: → 1466580
You need to log in before you can comment on or make changes to this bug.