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

add tests for php5.6 #418

Merged
merged 7 commits into from
Mar 6, 2018
Merged

Conversation

bastelfreak
Copy link
Member

No description provided.

bastelfreak added a commit to bastelfreak/puppet-php that referenced this pull request Feb 17, 2018
software-properties-common is needed to add ppas on ubuntu. the
puppetlabs-apt module can manage the package. it is save to enable this,
because the module uses ensure_package: https://github.com/puppetlabs/puppetlabs-apt/blob/df40baebedf5c0c15e08f3ec78adfd760b1371ca/manifests/ppa.pp#L30

Without this, managing ppa repos at least on ubuntu 16.04 is impossible. I noticed this bug while testing with aceptance tests in voxpupuli#418
@bastelfreak bastelfreak force-pushed the acceptancephp56 branch 2 times, most recently from c2f1975 to d2ce25b Compare February 17, 2018 13:16
@bastelfreak
Copy link
Member Author

bastelfreak commented Feb 17, 2018

The new spec file alone is working. Our module doesn't support the upgrade/downgrade of the php version, and beaker-rspec executes all tests in the same container. This seems to work in other modules like jenkins??

@bastelfreak bastelfreak changed the title [WIP]add tests for php5.6 add tests for php5.6 Feb 22, 2018
@bastelfreak bastelfreak force-pushed the acceptancephp56 branch 2 times, most recently from 4fdf8c8 to 4452c22 Compare February 22, 2018 23:46
@ekohl
Copy link
Member

ekohl commented Feb 23, 2018

If we do go this way, I'd prefer the following in .sync.yml

---
.travis.yml:
  docker_sets:
    - set: docker/ubuntu-16.04
      options:
        script: 'bundle exec rspec spec/acceptance/php56_spec.rb'
    - set: docker/ubuntu-14.04
      options:
        script: 'bundle exec rspec spec/acceptance/php56_spec.rb'
    - set: docker/centos-7
      options:
        script: 'bundle exec rspec spec/acceptance/php56_spec.rb'
    - set: docker/debian-8
      options:
        script: 'bundle exec rspec spec/acceptance/php56_spec.rb'
    - set: docker/ubuntu-16.04
      options:
        script: 'bundle exec rspec spec/acceptance/php_spec.rb'
    - set: docker/ubuntu-14.04
      options:
        script: 'bundle exec rspec spec/acceptance/php_spec.rb'
    - set: docker/centos-7
      options:
        script: 'bundle exec rspec spec/acceptance/php_spec.rb'
    - set: docker/debian-8
      options:
        script: 'bundle exec rspec spec/acceptance/php_spec.rb'

end

case default[:platform]
when %r{16.04}
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this the same as 14.04? Would it be better to test for the Debian OS family or at least Ubuntu?

describe package('php-fpm') do
it { is_expected.to be_installed }

case default[:platform]
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to set the expected package name in a case statement (with a let block?) and then have the same describe block with a variable

Copy link
Member Author

Choose a reason for hiding this comment

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

implemented it, just without the let block (didn't work)

@bastelfreak bastelfreak force-pushed the acceptancephp56 branch 3 times, most recently from ba844bc to 9650477 Compare March 6, 2018 12:49
Copy link
Member

@foxxx0 foxxx0 left a comment

Choose a reason for hiding this comment

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

looks good to me

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I'm thinking about environment variables rather than explicit filenames. You can do describe "case", if/unless: ... do and set the variables via Travis.


describe 'with specific php version' do
case default[:platform]
when %r{16.04}
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to just check for Ubuntu here?

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah would work as well

packagename = 'php5.6-fpm'
when %r{14.04}
packagename = 'php5.6-fpm'
when %r{7}
Copy link
Member

Choose a reason for hiding this comment

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

Is this Debian 7 or CentOS 7?

packagename = 'php5.6-fpm'
when %r{7}
packagename = 'php-fpm'
when %r{8}
Copy link
Member

Choose a reason for hiding this comment

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

I'm guessing Debian?

@@ -0,0 +1,45 @@
require 'spec_helper_acceptance'

describe 'with specific php version' do
Copy link
Member

Choose a reason for hiding this comment

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

How about a check for php --version?

@bastelfreak bastelfreak force-pushed the acceptancephp56 branch 2 times, most recently from 8e1e38b to dd275af Compare March 6, 2018 14:47
package = 'php5-fpm'
when %{centos}
package = 'php-fpm'
when %{'debian'}
Copy link
Member

Choose a reason for hiding this comment

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

Are the quotes needed here? should it be a regex too?

it { is_expected.to be_enabled }
end
describe command('php --version') do
its(:stdout) { is_expected.to match %r{5.6} }
Copy link
Member

Choose a reason for hiding this comment

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

Should the dot in 5.6 be escaped since it's a regex char?

@bastelfreak bastelfreak force-pushed the acceptancephp56 branch 6 times, most recently from 592c464 to fcccf06 Compare March 6, 2018 15:48
@bastelfreak bastelfreak force-pushed the acceptancephp56 branch 3 times, most recently from b0d1cb2 to 761eec2 Compare March 6, 2018 16:21
@bastelfreak
Copy link
Member Author

finally green

@bastelfreak bastelfreak merged commit b98a6a5 into voxpupuli:master Mar 6, 2018
@bastelfreak bastelfreak deleted the acceptancephp56 branch March 6, 2018 16:40
motivator pushed a commit to bigcommerce/puppet-php that referenced this pull request Oct 5, 2020
software-properties-common is needed to add ppas on ubuntu. the
puppetlabs-apt module can manage the package. it is save to enable this,
because the module uses ensure_package: https://github.com/puppetlabs/puppetlabs-apt/blob/df40baebedf5c0c15e08f3ec78adfd760b1371ca/manifests/ppa.pp#L30

Without this, managing ppa repos at least on ubuntu 16.04 is impossible. I noticed this bug while testing with aceptance tests in voxpupuli#418
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants