The committers are authorized under a signed CLA.

• ✅ Bruno PIERRE (a529f5e)

Thanks for opening a pull request! It seems we forgot to change our use of splice when removing it from the jQuery object. We did take care of sort and push, just not splice. I'm surprised our tests didn't catch this, but we're going to need to add a test.

Something like the case in the jsbin will work fine to get duplicates on the jQuery object (i.e. jQuery('body div').find('div')). A relatively pointless call to find as it won't find anything new, but it does the job here of inserting duplicates. That said, there are a lot of divs on the test page, so it would be good to limit it to smaller selection. Something like jQuery("#moretests div").find("div") should work.

Also, splice should be its own var file, just like with sort

Thanks!

@bubbatls Hey, are you interested in addressing Timmy's concerns?

@bubbatls Ping. This looks like an issue we'd like to see fixed but to land your PR we need to have Timmy's concerns addressed. If you don't have time for this, please let us know and we'll likely fix this by ourselves.

I'm sorry, I was in holiday and after holidays there is a lot of work to do...
Yes I could fix this using your kind of var files, just let me few days.

Thanks

@bubbatls Sure, no problem. Let us know if you hit any roadblocks.

yes, the test you write is the most unit one about this issue, and the method name is uniqueSort so, checking the "unique" is completely relevant.

But I agree that another one covering more code by doing the .find('div').find('div') is worth being written for potential future regression.

This test should be written in the test/unit/selector.js file?

This test should be written in the test/unit/selector.js file?

Yes, the both tests should be written in test/unit/core.js test/unit/selector.js but in separate QUnit.test blocks as they're testing two different things.

EDIT: I made a typo and I mentioned test/unit/core.js instead of the proper test/unit/selector.js; I fixed the comment now.

I'm sorry but I spend some time to try to understand how to run the tests before modifying anything. But I have always the error :
test/data/qunit-fixture.js net::ERR_ABORTED 404 (Not Found)
And I don't understand what I'm doing wrong.

How are you running the tests?

You need to build before running the tests as the build process creates a few test files and also builds jQuery and the built version is what gets tested by default. Run npm run build before running the tests.

Thank you, I'm sorry I'm new to npm. In fact there was a bug in an existing unit test for all that. So I've fixed it

Nice! thanks you all :)