Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Fix zap tests #21610

Closed
wants to merge 1 commit into from
Closed

[WIP] Fix zap tests #21610

wants to merge 1 commit into from

Conversation

claui
Copy link
Contributor

@claui claui commented Jun 1, 2016

Summary

The zap command has test coverage issues as pointed out in #21607.
This WIP branch is to discuss, investigate, and ultimately fix those issues.

I have added a temporary rake test:zap command for convenience.

How to run the isolated test

  1. Go to your local repo.
  2. Check out the branch:
    git fetch [email protected]:claui/homebrew-cask.git refs/heads/fix-zap-tests && git checkout FETCH_HEAD
    (This will leave all your existing remotes/branches alone; it will not even add a remote.)
  3. Run the isolated test:
    bundle exec rake test:zap

@claui claui added discussion core Issue with Homebrew itself rather than with a specific cask. on hold labels Jun 1, 2016
@claui
Copy link
Contributor Author

claui commented Jun 1, 2016

For starters, I have uncommented the first few lines of the failing test:

  it "dispatches both uninstall and zap stanzas" do
    with_zap = Hbc.load('with-zap')

    # shutup do
      Hbc::Installer.new(with_zap).install
    # end
  # […]
  end

and this already fails. Disarming the sudo part does not help:

    # shutup do
      Hbc::Installer.new(with_zap,
        command: Hbc::NeverSudoSystemCommand).install
    # end

In either case, /usr/sbin/installer causes an error:

installer: Error the package path specified was invalid:
'/var/folders/9k/72bcsqxd55q00scw81pt2f6h0000gp/T/homebrew_tests20160601-35211-cx84y/prefix/TestCaskroom/with-zap/1.2.3/MyFancyPkg/Fancy.pkg'.

I have double-checked whether Fancy.pkg is actually there.
Also, when I run installer manually on Fancy.pkg, I get the same error.

Any ideas?

@fanquake
Copy link
Contributor

fanquake commented Sep 5, 2016

What's the status here? Should this be rebased and Travis re-run?

@claui
Copy link
Contributor Author

claui commented Sep 5, 2016

@fanquake Good point. This is not really urgent but I agree in that we shouldn’t ever take test coverage lightly.

I’ll rebase by the end of the week if no one else does.

@claui claui self-assigned this Sep 5, 2016
@jawshooah
Copy link
Contributor

@claui would you like to try applying these changes on Homebrew/brew?

@jawshooah jawshooah closed this Oct 20, 2016
@claui
Copy link
Contributor Author

claui commented Oct 20, 2016

@jawshooah As soon as I’m through my backlog, I will reopen and apply as suggested!

@jawshooah
Copy link
Contributor

Thanks, no rush! Just wanted to get our PRs to Inbox Zero since we're so tantalizingly close! 😲

@miccal miccal removed core Issue with Homebrew itself rather than with a specific cask. discussion on hold labels Dec 23, 2016
@Homebrew Homebrew locked and limited conversation to collaborators May 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants