Fix test expectations #891
Conversation
Fixes a regression from d73507e which accidentally broke this.
|
@bors-servo r+ Thanks! |
|
@photoszzt: 🔑 Insufficient privileges: Not in reviewers |
|
@bors-servo r=photoszzt |
|
📌 Commit f4763bc has been approved by |
Fix test expectations r? @photoszzt cc @tmfink: I had to revert the target features commit because there were a bunch of compilation failures in the test expectations (we had another, earlier regression that accidentally made it so that we weren't compiling and running the test expectations in CI, so we didn't catch the target features PR's regressions).
|
@fitzgen, what are next steps? Should I open another pull request? What regressions did I introduce? I did not observe any test failures locally. |
|
@tmfink Have you tried to run the following: |
|
Yes, let's open a new pull request to reland it. As @photoszzt points out, because the test expectations changed, we need to retest that they can still compile, and that their layout unit tests still pass. This is done by changing into the I haven't dug into why the test expectations are failing to compile, yet, and I don't have time at the moment. I can help out on Tuesday, though (or answer questions you ask in the new PR before then at that time). Good luck! |
|
@fitzgen I think running cargo test in tests/expectations should be documented somewhere. At least I don't know about it until you told me. |
|
☀️ Test successful - status-travis |
|
Yeah definitely... I thought we had something in |
|
@photoszzt @fitzgen, I always ran |
|
I see that running |
r? @photoszzt
cc @tmfink: I had to revert the target features commit because there were a bunch of compilation failures in the test expectations (we had another, earlier regression that accidentally made it so that we weren't compiling and running the test expectations in CI, so we didn't catch the target features PR's regressions).