search

jhoblitt / puppet-selenium

Manages Selenium Server 2 - Standalone and Grid
Other
20 stars 49 forks source link

fixing tests and upgrading to latest rpsec #36

Closed tphoney closed 9 years ago

tphoney commented 9 years ago

@jhoblitt Travis tests were failing due to future parser on the logrotate module being used. (https://github.com/rodjek/puppet-logrotate/issues/46) I have updated to a later version of rspec. Acceptance tests were failing, added "nocheckcertificate => true" to fix the issue.

jhoblitt commented 9 years ago

@tphoney What platform were the acceptance tests failing on? I have not seen the certificate error that folks have reported and don't really want to disable certificate checking the acceptance tests.

In rspec3, is_expected.to is equivalent to should, which is not deprecated in the implicit receiver form. I prefer should as it's few characters both to type and read.

tphoney commented 9 years ago

The tests were failing against the default node. This was a fresh checkout with the latest gems.

I was having a play with http://yujinakayama.me/transpec/ the changes checked in. Where the recommendations it suggested.

@jhoblitt Thanks for having a look at this.

jhoblitt commented 9 years ago

I wasn't aware of transpec -- I should definitely look into it. The docs suggest that the "oneliner" conversion can be disabled. https://github.com/yujinakayama/transpec#conversions-enabled-by-default

I will try to reproduce the certificate validation error.

jhoblitt commented 9 years ago

I've reproduced the certificate error with the default centos 6.4 nodeset. It is not present with centos 7.0 -- I'm not yet sure if this is due to different versions of wget, changes to the google certificate, or the installed CA set.

tphoney commented 9 years ago

agreed, i just saw it with the default nodeset. sorry if i am just creating churn for you.

jhoblitt commented 9 years ago

@tphoney No need to apologize at all! Any contribution is greatly welcomed.

The acceptance nodesets have needed to be updated for awhile. I've been trying to hold off on a round test infrastructure updates across all of my modules until a stable release of rspec-puppet/puppetlabs_spec_helper is compatible with puppet 4.0.0.

I've pushed up a branch with updated gem versions and nodeset files. https://github.com/jhoblitt/puppet-selenium/tree/maint/test_infra

The centos-6.6 set in this branch passes for me without disabling the certificate check. Could you give it a try and let me know? I suspect, but have not tested, that the wget and/or ca-certificates packages in ~ el 6.4 dislike the SAN certificate currently being returned by https://selenium-release.storage.googleapis.com .

I'm not enthusiastic about disabling the cert check without some sort of digest verification of the jar file. Do you feel it would be reasonable to handle this as a docfix and add a note to the README about certificate validation errors?

tphoney commented 9 years ago

That all seems totally sane, i will check out the code, and see if it all passes. Thanks again for your patience.

jhoblitt commented 9 years ago

I've gone ahead and merged #37 . Would you be interested in drafting some words for the README about certificate validation errors or retrying the transpec update without modifying the implicit receiver form of should?

tphoney commented 9 years ago

i was testing that :D, i will draft something for the readme. I will explain what the error is, and give your explanation of the error.

tphoney commented 9 years ago

The tests ran through fine, i will look at the readme and rspec change and i will close this pr

Thanks again