- Notifications
You must be signed in to change notification settings - Fork20.6k
Selector: Port Sizzle tests to jQuery#4406
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
I ported many more tests, pushed in new commits. Still a lot to go but we're getting closer. |
I added a few more commits. It's not 100% ready yet but it's close so I'd appreciate a review. I'll finish remaining pieces & address feedback when I'm back from vacation. |
This is a lot to review. Honestly, I'm okay with merging when you feel it is ready and then we can increment in smaller PRs from there. |
mgol commentedJun 24, 2019 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
As discussed at the meeting today, I'll merge it soon (tomorrow or on Wednesday) unless someone wants to have a look. Some final remarks/questions from me (you can review them instead if you want):
|
Apart from porting most Sizzle tests to jQuery (mostly to its selector module),this commit fixes selector-native so that a jQuery custom compilation thatexcludes Sizzle passes all tests as well.
Uh oh!
There was an error while loading.Please reload this page.
Summary
This PR ports Sizzle tests to jQuery. Reviewing by commit might be the best. Sizzle tests are located in three files named
extending.js
,utilities.js
&selector.js
. TODO list:extending.js
: these are only tests for custom pseudos which we'll be removing so I haven't backported them.utilities
.js: I backported everything that wasn't already present with the exception ofSizzle.attr
a.k.a.jQuery.find.attr
. This is an internal API & I think jQuery attribute tests are thorough enough that we don't need to add these tests. Let me know if you disagree.selector.js
.Checklist
New tests have been added to show the fix or feature worksIf needed, a docs issue/PR was created athttps://github.com/jquery/api.jquery.com