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

ci(platform): add arch-base-latest & split suites across Travis instances #43

Merged
merged 8 commits into from
Sep 30, 2019

Conversation

myii
Copy link
Member

@myii myii commented Sep 23, 2019

@myii myii force-pushed the chore/standardise-structure branch from a8d4176 to 11af24c Compare September 23, 2019 00:54
@myii
Copy link
Member Author

myii commented Sep 23, 2019

@dafyddj Another issue that could be covered at the same time:

  • Many of our formulas have multiple tests suites but we tend to use only run one suite per instance in Travis.
  • Now that we have seven instances available, can we split testing the three suites across them? I.e.:
    • Two of the instances testing dev_server.
    • Two of the instances testing install_binary.
    • Two of the instances testing prod_server.
    • The remaining instance could be kept the same, testing all three suites (and running that one first, so that it doesn't delay the overall run).

@myii myii force-pushed the chore/standardise-structure branch 2 times, most recently from eb7a407 to 6c73c5c Compare September 23, 2019 14:16
@myii myii force-pushed the chore/standardise-structure branch from 9907631 to 6dd656f Compare September 25, 2019 18:43
@myii
Copy link
Member Author

myii commented Sep 25, 2019

@dafyddj So I've prepared a comparison, before and after splitting the suites across the various instances:

The differences are significant. If you're OK with it, I'll add that to this PR.

@myii myii changed the title ci(platform): add arch-base-latest ci(platform): add arch-base-latest & split suites across Travis instances Sep 27, 2019
@myii
Copy link
Member Author

myii commented Sep 27, 2019

@dafyddj I've added the commit to split the suites in Travis, so this is ready for review.

@myii myii requested a review from dafyddj September 27, 2019 02:50
@myii myii force-pushed the chore/standardise-structure branch from f6762a6 to c82034a Compare September 27, 2019 04:00
@myii
Copy link
Member Author

myii commented Sep 27, 2019

@dafyddj There's bad news on the horizon. As I thought I was done with this PR, all of the instances started failing (when they were working fine in my fork yesterday). Dug down and found out that the latest inspec gem that has just been released (v4.17.7) is resulting in undefined method errors. Added a commit to restrict it to the previous version (v4.16.0) and everything is working again.

A summary of the errors avoided (for the time being):

https://travis-ci.com/saltstack-formulas/vault-formula/jobs/239671364#L2219-L2220 (prod_server):

>>>>>>     Failed to complete #verify action: [undefined method `file' for #<#<Class:0x000055dca642a3c8>:0x000055dca642a2d8>
Did you mean?  fail] on prod-server-ubuntu-1804-2019-2-py3

https://travis-ci.com/saltstack-formulas/vault-formula/jobs/239671365#L1925-L1926 (dev_server):

>>>>>>     Failed to complete #verify action: [undefined method `file' for #<#<Class:0x000055cdc9bcb5f0>:0x000055cdc9bd26e8>
Did you mean?  fail] on dev-server-fedora-30-2019-2-py3

https://travis-ci.com/saltstack-formulas/vault-formula/jobs/239671366#L1515-L1520 (install_binary):

     ×  should not be installed
     undefined method `sysv_service' for #<#<Class:0x0000558e533124a0>:0x0000558e533340a0>
     ×  should not be enabled
     undefined method `sysv_service' for #<#<Class:0x0000558e533124a0>:0x0000558e533340a0>
     ×  should not be running
     undefined method `sysv_service' for #<#<Class:0x0000558e533124a0>:0x0000558e533340a0>

Copy link
Contributor

@dafyddj dafyddj left a comment

Choose a reason for hiding this comment

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

WRT splitting the suites, prod_server is really the important test suite. The other two test suites mainly test formula logic so only really need running once. I've made some suggested changes to that effect.

Other than that, happy with it, and thanks for helping to improve the formula 👍🏻

.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
.travis.yml Outdated Show resolved Hide resolved
Co-Authored-By: Dafydd Jones <dafydd@techneg.it>
@myii
Copy link
Member Author

myii commented Sep 30, 2019

@dafyddj Thanks for the review, I've committed the suggestions as a batch. Now due to these repo's settings (as outlined in #45), we can't merge this because of an org-wide issue we're currently having with opensuse. I'll probably fix that in a separate PR, rebase this PR on top and merge it once it's successful.

@myii myii merged commit a5ecc50 into saltstack-formulas:master Sep 30, 2019
@saltstack-formulas-travis

🎉 This PR is included in version 1.2.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

myii pushed a commit to myii/ssf-formula that referenced this pull request Oct 1, 2019
# [1.8.0](v1.7.0...v1.8.0) (2019-10-01)

### Features

* **vault:** limit commented instances to empty suite ([eee355b](eee355b))
* **vault:** split suites across instances leaving one running all ([e7483a3](e7483a3)), closes [/github.com/saltstack-formulas/vault-formula/pull/43#issuecomment-533936364](https://github.com//github.com/saltstack-formulas/vault-formula/pull/43/issues/issuecomment-533936364)
* **vault:** use specific Gemfile to restrict `inspec` version ([a9b7ff9](a9b7ff9))
* **vault): ci(travis:** apply suggestions from code review ([54ea2fb](54ea2fb))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants