Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upWPT tests now show debug ouput for passing tests as well #15124
Conversation
highfive
commented
Jan 20, 2017
|
Heads up! This PR modifies the following files:
|
|
I guess I don't exactly know what you want, but this seems like a pretty surprising change; the point of the grouping formatter was to have as little output as possible. Maybe a simpler alternative would be just to switch to using |
|
@jgraham The default formatter is in an interesting position. On one hand, minimizing the output for passing tests is often valuable. On the other hand, we regularly have contributors adding println! and debug! to code that executes during a passing test and becoming confused when no output appears. --log-mach is completely undiscoverable, so I lean towards addressing that problem. Maybe we could change the default based on whether a singles test is being run versus multiple ones. |
|
That seems very possible. Just change the default in |
|
@cynicaldevil Want to revert the changes in this PR and make the one suggested in the previous comment instead? |
|
Sure, I'll do it tomorrow. |
|
Updated the PR. |
|
Huh, that doesn't get overridden by https://github.com/cynicaldevil/servo/blob/3fd5122930e2d009769ac455e0d5ce4768b5a0af/tests/wpt/run.py#L32-L34 ? |
|
Using print messages I found that |
|
What if we make the code clearer by moving the other logger into an else branch? |
|
@jdm done. |
|
@bors-servo: r+ |
|
|
WPT tests now show debug ouput for passing tests as well <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix (partially) #12808 <!-- Either: --> @jdm I thought it would be better to make a PR for my local changes anyway, since you wanted it :) <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15124) <!-- Reviewable:end -->
|
@bors-servo: r+ |
|
|
|
|
@bors-servo r- try- clean |
|
@bors-servo r=jdm |
|
|
|
Let's try syncing homu |
|
@cynicaldevil please find me on IRC when you can? Thanks |
|
@bors-servo r=jdm |
|
|
WPT tests now show debug ouput for passing tests as well <!-- Please describe your changes on the following line: --> --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix (partially) #12808 <!-- Either: --> @jdm I thought it would be better to make a PR for my local changes anyway, since you wanted it :) <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/15124) <!-- Reviewable:end -->
|
|
|
@bors-servo retry |
|
|
|
|
cynicaldevil commentedJan 20, 2017
•
edited by jdm
./mach build -ddoes not report any errors./mach test-tidydoes not report any errors@jdm I thought it would be better to make a PR for my local changes anyway, since you wanted it :)
This change is