-
Notifications
You must be signed in to change notification settings - Fork 3.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Initial test for resource timing #402
Initial test for resource timing #402
Conversation
Critic review: https://critic.hoppipolla.co.uk/r/393 This is an external review system which you may optionally use for the code review of your pull request. |
test_resource_cached.html is generated inconsistent results (could be a Google canary error?) |
@plehegar there's been no action on this for 6 months, and you have review comments waiting over at https://critic.hoppipolla.co.uk/r/393 |
@plehegar radio check |
@igrigorik @plehegar This PR needs some love. What are the next steps? This may be one where some assistance from Xiaquan |
@siusin looked at this PR and concluded that it overlaps with what we have in the repo. Proposal is to close down the PR with no action. @igrigorik ? |
sgtm, closing. |
These tests clearly have additional coverage missing from existing tests. They cover clearing the buffer which is something the existing tests don't cover. There is some overlap, but I'm not clear that is a good reason to shut this PR down. |
Sorry, I was looking at a wrong branch when I did the review last time. My WPT repo got messed up after I merged these commits to the latest :( @toddreifsteck is correct. I think it's reasonable to keep the following tests, although some of them need to be modified:
These two tests can be removed for overlaps:
I'll submit the updated tests later. |
(goal is to close this PR and open one or more new ones with the proper test) |
Updated as pull request #4266 :
|
subsumed by other PRs |
This is just an initial test to adjust all of the tests before moving them all.