HOLD: Add tests for brew cask search when casks have the same token

[claui]

This PR adds failing tests for just one more edge case I’ve encountered.

Issue

When two or more Casks (from separate repos) have the same token, the brew cask search command can get a bit confused.

I’m not really sure if we’re committed to supporting this in the first place. However, we have a method Cask::CLI::nice_listing which suggests that we are.

For this discussion, let’s suppose a user has tapped into a repo called other/repo which happens to carry another firefox. (This PR includes a Cask like this in case you want to have a look.)

Current behavior

Now that either repo has a firefox, the command brew cask search fire yields:

==> Partial matches
caskroom/cask/firefox  caskroom/cask/firefox   mediafire-desktop   multifirefox

with caskroom/cask/firefox listed twice while other/repo/firefox is missing.

Similarly, brew cask search firefox yields:

==> Exact match
caskroom/cask/firefox
==> Partial matches
multifirefox

with only one match, but other/repo/firefox still missing.

Proposed behavior

Instead, I’d expect brew cask search fire to yield:

==> Partial matches
caskroom/cask/firefox  other/repo/firefox      mediafire-desktop   multifirefox

and brew cask search firefox to yield something like this:

==> Exact match
caskroom/cask/firefox
==> Partial matches
other/repo/firefox     multifirefox

Please do NOT merge

I only created this PR to have a place for the failing tests … and to kindly ask for your thoughts about them.

The upcoming version of brew cask search already implements what the Proposed section says. Still, I’d love some feedback from the @caskroom/maintainers … does the proposed behavior make sense?

[radeksimko]

Definitely 👍 for better test coverage! :)

Nonetheless, do you have any particular reasons behind using minitest instead of RSpec (except there's lots of tests currently written in it)?

Check out this PR: #7880 and this change 58d4c74

[claui]

Thanks for the pointer @radeksimko!

@rolandwalker had already told me about the plans of transitioning to RSpec … I wasn’t aware though that we’re already free to use it 😊

[rolandwalker]

We are already supposed to use RSpec, but I need to be taught and/or led into it.

I have only minimal understanding of minitest, and just fake it by copying other examples.

[rolandwalker]

As to the substance of the proposal, it sounds good and certainly fixes a bug. Some people might prefer to always hide the canonical Caskroom tap, eg

$ brew cask search firefox
==> Exact match
firefox
==> Partial matches
other/repo/firefox     multifirefox

so you might want to gather opinions. My vote would go for your original proposal.

We do avoid creating colliding tokens anywhere across our repos, but a user has the freedom to create one and we ought to support that.

It's worth noting that, however the token is specified at the CLI, brew cask install will stage the distribution at /opt/homebrew-cask/Caskroom/<token>/<version>. So other/repo/firefox wants to be a replacement for caskroom/cask/firefox. You could argue that this is a feature, or a bug. I don't think any of us has thought it through completely.

An alternative arrangement which would allow multiple firefox tokens to coexist would be to stage at /opt/Caskroom/homebrew-<repo>/<token>/version. I'm not certain that would be beneficial, but it has been raised before. (I cannot find the relevant issue).

[radeksimko]

I like the idea of more different versions of the same app from different repos and I think we should be supporting it, but it's just worth noticing, that the whole problem gets a bit more complicated with shared namespace in [~]/Applications/ and even more complicated with pkg installers.

[fanquake]

Looks interesting. Will take a look over the next day or two.

On Tuesday, December 9, 2014, Radek Simko [email protected] wrote:

I like the idea of more different versions of the same app from different
repos and I think we should be supporting it, but it's just worth noticing,
that the whole problem gets a bit more complicated with shared namespace in
[~]/Applications/ and even more complicated with pkg installers.

Reply to this email directly or view it on GitHub
#7906 (comment)
.