Fix REPL test result uniqueness

[Moelf]

Fixes #32377

Makes the behavior match whats in production code? Since REPL can come from Package and module.

julia/stdlib/REPL/src/REPL.jl

Line 363 in 67cdc55

return unique!(map(completion_text, ret)), partial[range], should_complete

[vtjnash]

I think one of the intents of this test is to make sure that the completions aren't returning non-unique duplicates

[KristofferC]

The completions can have duplicates which is why we call unique! on it in the REPL code itself, #28692, #28694.

[vtjnash]

Yes, and this is the test you added in #28694 to check that the list does not have duplicates

[KristofferC]

The test added there assumes that test_completion does the same as complete_line. However, right now, only complete_line does the unique! call. So the test is at least faulty with the current map_completion_text.

[Moelf]

But I can see that with this change, the count( ...) == 1 part is obsolete.

[KristofferC]

Yes, that is right. The way things are written now, it is a bit hard to test this properly.

[tgflynn]

I think one of the intents of this test is to make sure that the completions aren't returning non-unique duplicates

Why would that be a problem ?

[Moelf]

bump

[tgflynn]

Doesn't the REPL always obtain completions through complete_line (where the uniqueness problem was fixed in PR #28694) ?

If that's the case then it seems like either this PR should be merged to make this test work like complete_line or this test should be removed completely and replaced by a test for complete_line.

[tgflynn]

The weird thing about PR #28694 is that it only changed code in complete_line but the test it adds doesn't test complete_line.

[Moelf]

is this still relevant?

[KristofferC]

Yes, let's re-run CI and merge this.

[vtjnash]

I think we can't merge this as-is since it defeats the purpose of the existence of this test. We either need to delete this test (and reopen #26930), or figure out how to fix and/or test it properly.

[vtjnash]

Superseded by #36506