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

(PDK-385) Support package testing on OSX #225

Merged
merged 1 commit into from
Aug 8, 2017

Conversation

james-stocks
Copy link

@james-stocks james-stocks commented Aug 7, 2017

Add package install for OSX (beaker does not have a helper for this, yet. See https://tickets.puppetlabs.com/browse/BKR-1109 for having a single helper for all platforms implemented).

I'm unhappy with the amount of code here when beaker and vanagon should provide a 2-liner method to get a .dmg from the build server and install it on the workstation host.

Tested against a number of builds (including per-commit builds and tag builds). Also tested that this works with the LOCAL_PKG option to point to a local .dmg file instead of one on the build server

@james-stocks james-stocks requested review from scotje and DavidS August 7, 2017 15:53
@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.774% when pulling 7cb7244 on james-stocks:PDK-385 into 8dcf53c on puppetlabs:master.

build_yaml = open("http://#{ENV['BUILD_SERVER']}/pdk/#{ENV['SHA']}/repos/#{ENV['SHA']}.yaml").read
build_yaml = YAML.load(build_yaml)
artifact = build_yaml[:platform_data][workstation['platform']][:artifact][1..-1]
pkg = "http://#{ENV['BUILD_SERVER']}/pdk/#{ENV['SHA']}/repos#{artifact}"
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Jenkins provides the required build string here as SUITE_VERSION.
If I use that then I can remove this yaml and string slicing code.
That would mean one more ENV for a local user to have to manage; but that's better than having to rely on this code...

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.774% when pulling 7706318 on james-stocks:PDK-385 into 8dcf53c on puppetlabs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.774% when pulling 3900b89 on james-stocks:PDK-385 into 8dcf53c on puppetlabs:master.

@scotje
Copy link
Contributor

scotje commented Aug 7, 2017

Vanagon now helpfully writes a SHA.build_metadata.json to the artifacts directory which contains the SUITE_VERSION, so we can use that to calculate the needed version values without requiring SUITE_VERSION.


if workstation['platform'] =~ %r{windows}
pkg ||= "http://#{ENV['BUILD_SERVER']}/pdk/#{ENV['SHA']}/repos/windows/pdk-x64.msi"
full_version = build_info['version']
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do all this in the Rakefile and actually set SUITE_VERSION only if unset. Would that be better?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be most helpful to use SUITE_VERSION if it's already set; and fall back to build_metadata.json if it's not set.

I'm unsure if I prefer the code being here or in the Rakefile. It might be better in the Rakefile in order to ensure we have all the essential data before executing beaker and allocating VMs

@scotje
Copy link
Contributor

scotje commented Aug 7, 2017

Oh and here is where the version-less MSI is getting created: https://github.com/puppetlabs/packaging/blob/master/tasks/ship.rake#L619-L642

@coveralls
Copy link

Coverage Status

Coverage remained the same at 90.774% when pulling f9a884d on james-stocks:PDK-385 into 8dcf53c on puppetlabs:master.

test_name 'Install pdk package on workstation host' do
workstation = find_at_most_one('workstation')

step 'Install pdk package' do
if ENV['LOCAL_PKG']
pkg = File.basename(ENV['LOCAL_PKG'])
scp_to(workstation, ENV['LOCAL_PKG'], pkg)
end
else
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This else looks wrong - if you're using LOCAL_PKG then you still need the build version string for generic_install_dmg to get the pkg name inside the dmg

@james-stocks
Copy link
Author

james-stocks commented Aug 8, 2017

@scotje I pushed a commit that moves the determination of SUITE_VERSION to Rakefile instead of doing it in the test code.
Also has a case where it can try to figure out SUITE_VERSION from the package filename if you're using LOCAL_PKG but didn't specify SUITE_VERSION. Note that SUITE_VERSION is needed for a LOCAL_PKG run because beaker needs to know the filename of the .pkg file inside the .dmg file; and that .pkg filename will have SUITE_VERSION in it

Let me know what you think - at least some of the commits on this PR can be squashed, but only if the current code is what we want to go with.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.23% when pulling 09bb530 on james-stocks:PDK-385 into 470cf22 on puppetlabs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.23% when pulling 09bb530 on james-stocks:PDK-385 into 470cf22 on puppetlabs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.23% when pulling 510584c on james-stocks:PDK-385 into 470cf22 on puppetlabs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.23% when pulling f122189 on james-stocks:PDK-385 into 470cf22 on puppetlabs:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 92.23% when pulling f122189 on james-stocks:PDK-385 into 470cf22 on puppetlabs:master.

@scotje scotje merged commit 47fb4ee into puppetlabs:master Aug 8, 2017
@chelnak chelnak added the maintenance Internal maintenance work that shouldn't appear in the changelog label Jan 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Internal maintenance work that shouldn't appear in the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants