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 upChange Performance interface's timing member to an attribute #23367
Conversation
highfive
commented
May 11, 2019
|
Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @jdm (or someone else) soon. |
highfive
commented
May 11, 2019
|
Heads up! This PR modifies the following files:
|
highfive
commented
May 11, 2019
|
|
|
@bors-servo try=wpt |
bors-servo
added a commit
that referenced
this pull request
May 13, 2019
Change Performance interface's timing member to an attribute <!-- Please describe your changes on the following line: --> Change Performance interface's timing member to an attribute --- <!-- 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 #23330 (GitHub issue number if applicable) <!-- Either: --> - [ ] These changes do not require tests because ___ - [ ] There are tests for these changes OR <!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.--> <!-- 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/23367) <!-- Reviewable:end -->
|
|
|
Looks like we'll need to figure out why the code at line 372 of components/script/dom/performance.rs is triggering in this case! You can run this test locally with |
|
I think I'm able to see the same thing: outputDoing some digging now. Also, I've built and run the project on a linode server and the test seems to be printing an error about x11/wayland which makes sense since there's no display manager running. Should I be concerned about that? Is there a supported way to run tests in a sorta headless mode? |
|
Unfortunately that's a very different error: You could try running with |
|
Ok cool, looks like with output |
|
Hmm actually I'm a bit confused. Where is the file
since I can't seem to find that |
|
You have been doing the right thing. The test harness server has special logic to handle URLS ending in .worker.js that turns them into HTML files that run a worker and handle all the test harness integration. |
|
@garyemerson As @jdm said, you have been doing the right thing. For As described in the documentation example, the test is written in You can read its documentation here. |
|
Ah I see, thanks for the clarification. That documentation link was helpful too. Could someone help me understand how the as well as |
Yes, so if there's an interface So, for the |
Yeah that's what's confusing, the test should fail but it doesn't, it passes. |
|
Yes, that is a known issue that I stumbled across recently: #23332 |
|
Should we move forward with this PR (perhaps comment out/remove test for now) or just hang back until #23332 is resolved? |
|
Yes, let's go ahead and make the expected results for that test CRASH (like this other test, for example). This will allow us to notice when #23332 fixes it. |
|
@garyemerson Are you planning to finish this work? |
|
Closing due to lack of activity. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
garyemerson commentedMay 11, 2019
•
edited
Change Performance interface's timing member to an attribute
./mach build -ddoes not report any errors./mach test-tidydoes not report any errorsThis change is