Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Merged
mgol merged 1 commit intojquery:masterfrommgol:sizzle-tests-backport
Jun 26, 2019

Conversation

mgol
Copy link
Member

@mgolmgol commentedMay 20, 2019
edited
Loading

Summary

This PR ports Sizzle tests to jQuery. Reviewing by commit might be the best. Sizzle tests are located in three files namedextending.js,utilities.js &selector.js. TODO list:

  • Review tests fromextending.js: these are only tests for custom pseudos which we'll be removing so I haven't backported them.
  • Review tests fromutilities.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.
  • Review tests fromselector.js.

Checklist

@mgolmgol added the Tests labelMay 20, 2019
@mgolmgol added this to the4.0.0 milestoneMay 20, 2019
@mgolmgolforce-pushed thesizzle-tests-backport branch fromd3a7cab tod459aadCompareMay 27, 2019 19:56
@mgol
Copy link
MemberAuthor

I ported many more tests, pushed in new commits. Still a lot to go but we're getting closer.

@mgol
Copy link
MemberAuthor

mgol commentedJun 9, 2019

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.

@timmywil
Copy link
Member

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.

@mgolmgol marked this pull request as ready for reviewJune 24, 2019 18:22
@mgol
Copy link
MemberAuthor

mgol commentedJun 24, 2019
edited
Loading

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):

  1. I updatedselector-native slightly so that it passes some backported tests it used to fail - some Sizzle logic wasn't backported there.
  2. Retest/unit/extending.js: it contains tests for custom pseudos which we'll soon drop so I haven't backported it
  3. Retest/unit/utilities.js: it tests a few APIs that we already have tets for on the jQuery side; it also contains tests forSizzle.attr, though. I assume they're not needed as jQuery attr tests should be enough.
  4. Tests usingSizzle.matchesSelector have been migrated by replacingSizzle.matchesSelector(selector, elem) byjQuery(elem).is(selector).Sizzle.matches tests weren't backported as it's a private API that no code in jQuery or Sizzle uses.
  5. Lots of tests using Sizzle have been migrated to usejQuery.find - should we document that method? If not, should we migrate those tests to usejQuery? Some queries need to be modified then. jQuery.find accepts as many as four parameters; do we want to support them all? Some tests are using all of them right now.
  6. Safari has surprisingly good support for CSS selectors that other browsers don't support. E.g.:not(complex selector) works in Safari. Therefore, I UA-sniff in tests & run some tests just in Safari OR if Sizzle is present (by checking forjQuery.find.compile as existing tests do).

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.
@mgolmgolforce-pushed thesizzle-tests-backport branch from22ad604 to2dda538CompareJune 26, 2019 18:48
@mgolmgol merged commit79b74e0 intojquery:masterJun 26, 2019
@mgolmgol deleted the sizzle-tests-backport branchJune 26, 2019 19:39
@locklockbot locked asresolvedand limited conversation to collaboratorsJan 11, 2020
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
Milestone
4.0.0
Development

Successfully merging this pull request may close these issues.

2 participants
@mgol@timmywil

[8]ページ先頭

©2009-2025 Movatter.jp