Wrench Reftests #695
Wrench Reftests #695
Conversation
|
@jrmuizel Cool! Did you want this stuff reviewed / merged now, or wait until the [WIP] is removed? |
|
I'll put a reviewable version up tomorrow. |
|
Can we have the |
| } | ||
|
|
||
| pub fn parse_reftests<F>(filename: &str, mut runner: F) where F: FnMut(Reftest) | ||
| { |
There was a problem hiding this comment.
non-conventional formatting here (and in other function definitions)
| let kind = match items.next() { | ||
| Some("==") => ReftestOp::Equal, | ||
| Some("!=") => ReftestOp::NotEqual, | ||
| _ => panic!() |
There was a problem hiding this comment.
nit: maybe just continue with a warning instead
| wrench.render(); | ||
| window.swap_buffers(); | ||
| let size = window.get_inner_size(); | ||
| gl::read_pixels(0, 0, |
There was a problem hiding this comment.
uh oh, we need to hide it behind WR's device abstraction
| gl::read_pixels(0, 0, | ||
| size.0 as gl::GLsizei, | ||
| size.1 as gl::GLsizei, | ||
| gl::RGB, gl::UNSIGNED_BYTE) |
There was a problem hiding this comment.
Can we use RGBA instead in order to avoid internal format conversion?
| pub fn run_reftests(wrench: &mut Wrench, window: &mut WindowWrapper, filename: &str) | ||
| { | ||
| parse_reftests(filename, |t: Reftest| | ||
| { |
|
This should be ready for review now. Only the top commit is relevant. Some of the others were need so that continuous integration will work. |
| let file = BufReader::new(&f); | ||
| for line in file.lines() { | ||
| let l = line.unwrap(); | ||
| if l.starts_with("#") { |
There was a problem hiding this comment.
not really needed here, the next continue would do
There was a problem hiding this comment.
actually, the next continue is checking for l.len()==0 so it's different
| @@ -54,6 +54,9 @@ use yaml_frame_reader::YamlFrameReader; | |||
|
|
|||
| mod yaml_frame_writer; | |||
| mod json_frame_writer; | |||
| mod scene; | |||
There was a problem hiding this comment.
this looks like a leftover, and it fails to build
| // strip the comments | ||
| let s = &l[0..l.find("#").unwrap_or(l.len())]; | ||
| let s = s.trim(); | ||
| if l.len() == 0 { |
There was a problem hiding this comment.
that is comparing l.len() and not s.len()? I misread it at first.
| let file = BufReader::new(&f); | ||
| for line in file.lines() { | ||
| let l = line.unwrap(); | ||
| if l.starts_with("#") { |
There was a problem hiding this comment.
actually, the next continue is checking for l.len()==0 so it's different
|
|
||
| // strip the comments | ||
| let s = &l[0..l.find("#").unwrap_or(l.len())]; | ||
| let s = s.trim(); |
There was a problem hiding this comment.
a simpler way, possibly, is: let s = l.split('#').next().unwrap().trim()
|
@bors-servo r+ |
|
📌 Commit bf3993b has been approved by |
|
This is awesome, thanks! |
Wrench Reftests This is my work-in-progress that adds reftesting functionality to wrench <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/webrender/695) <!-- Reviewable:end -->
|
☀️ Test successful - status-travis |
This is my work-in-progress that adds reftesting functionality to wrench
This change is