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(windows): test using GitHub Actions #70

Merged

Conversation

myii
Copy link
Member

@myii myii commented Aug 8, 2020

PR progress checklist (to be filled in by reviewers)

  • Changes to documentation are appropriate (or tick if not required)
  • Changes to tests are appropriate (or tick if not required)
  • Reviews completed

What type of PR is this?

Primary type

  • [build] Changes related to the build system
  • [chore] Changes to the build process or auxiliary tools and libraries such as documentation generation
  • [ci] Changes to the continuous integration configuration
  • [feat] A new feature
  • [fix] A bug fix
  • [perf] A code change that improves performance
  • [refactor] A code change that neither fixes a bug nor adds a feature
  • [revert] A change used to revert a previous commit
  • [style] Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc.)

Secondary type

  • [docs] Documentation changes
  • [test] Adding missing or correcting existing tests

Does this PR introduce a BREAKING CHANGE?

No.

Related issues and/or pull requests

Built upon the work done by @dafyddj in other formulas:

Brings in automated testing for Chocolatey as discussed in #60 (CC: @kartnico).

Describe the changes you're proposing

Add Windows testing for Chocolatey installations.

Pillar / config required to test the proposed changes

As provided.

Debug log showing how the proposed changes work

As seen in the GitHub Actions logs.

Documentation checklist

  • Updated the README (e.g. Available states).
  • Updated pillar.example.

Testing checklist

  • Included in Kitchen (i.e. under state_top).
  • Covered by new/existing tests (e.g. InSpec, Serverspec, etc.).
  • Updated the relevant test pillar.

Additional context

@dafyddj As mentioned in Slack, I haven't managed to get the Vagrant testing working yet so I'd appreciate it if you have any suggestions for that.

Using this PR as a precursor to providing Windows testing configuration via. the ssf-formula.

@myii myii requested review from javierbertoli and a team as code owners August 8, 2020 20:24
@pull-assistant
Copy link

pull-assistant bot commented Aug 8, 2020

Score: 1.00

Best reviewed: commit by commit


Optimal code review plan

     ci(windows): test using GitHub Actions

Powered by Pull Assistant. Last update 1040cc8 ... 1040cc8. Read the comment docs.

@myii myii requested a review from dafyddj August 8, 2020 20:24
@myii myii force-pushed the ci/add-windows-testing-via-github-actions branch from 185d734 to 992eb48 Compare August 8, 2020 21:27
@myii
Copy link
Member Author

myii commented Aug 11, 2020

After feedback in the Formulas' working group meeting, tested some modifications in another branch and getting much better performance now:

@dafyddj mentioned that he would be able to add Chocolatey to the pre-salted image used by kitchen.vagrant.yml, so this PR will be checked again locally once that's done.

Also need to add the relevant documentation to README.rst.

As for the debian-10 failure, that's unrelated to this PR. Appears to be an issue with that particular snap but would need to dig down further, if I can make time. Otherwise, may have to just disable it in the test pillar.

@dafyddj
Copy link
Contributor

dafyddj commented Aug 14, 2020

@myii uploaded a new Vagrant box version. Do you want to try v2020.8.198?

@myii myii force-pushed the ci/add-windows-testing-via-github-actions branch from 992eb48 to 1040cc8 Compare August 25, 2020 20:45
@myii
Copy link
Member Author

myii commented Aug 25, 2020

@myii uploaded a new Vagrant box version. Do you want to try v2020.8.198?

Just confirming here that the new box works -- thanks. I've updated the PR accordingly, including removing unnecessary installations to make the Windows testing faster. So this PR should be ready for merging. The debian-10 failure is unrelated.

CC: @dafyddj @javierbertoli.

provisioner:
salt_install: bootstrap
salt_bootstrap_options: -pythonVersion 3
init_environment: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
init_environment: ""

Copy link
Contributor

Choose a reason for hiding this comment

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

Would suggest just removing the unneeded lines.

Comment on lines 13 to 14
provisioner:
init_environment: ""
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
provisioner:
init_environment: ""

Copy link
Contributor

Choose a reason for hiding this comment

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

And here.

@dafyddj
Copy link
Contributor

dafyddj commented Aug 28, 2020

Would also suggest changing the platform name to just windows for consistency.

@myii
Copy link
Member Author

myii commented Mar 26, 2021

Updated and finalised using myii/ssf-formula#304.

@myii myii force-pushed the ci/add-windows-testing-via-github-actions branch from ccd065b to c6e1380 Compare March 26, 2021 12:11
```
Offenses:

test/integration/default/controls/pkgs_spec.rb:27:5: W: [Corrected] Lint/SymbolConversion:
Unnecessary symbol conversion; use iotop: instead.
    'iotop': ''
    ^^^^^^^
test/integration/default/controls/pkgs_spec.rb:33:5: W: [Corrected] Lint/SymbolConversion:
Unnecessary symbol conversion; use alien: instead.
    'alien': '8.95-8.fc29',
    ^^^^^^^
test/integration/default/controls/pkgs_spec.rb:34:5: W: [Corrected] Lint/SymbolConversion:
Unnecessary symbol conversion; use iotop: instead.
    'iotop': '0.6-18.fc29'
    ^^^^^^^
test/integration/default/controls/pkgs_spec.rb:46:5: W: [Corrected] Lint/SymbolConversion:
Unnecessary symbol conversion; use alien: instead.
    'alien': '8.95',
    ^^^^^^^
test/integration/default/controls/pkgs_spec.rb:48:5: W: [Corrected] Lint/SymbolConversion:
Unnecessary symbol conversion; use iotop: instead.
    'iotop': '0.6-'
    ^^^^^^^
```
@myii myii force-pushed the ci/add-windows-testing-via-github-actions branch from c6e1380 to f7a6fcf Compare March 26, 2021 12:18
@myii myii force-pushed the ci/add-windows-testing-via-github-actions branch from a443b4e to bf0e39c Compare March 26, 2021 14:36
@myii myii force-pushed the ci/add-windows-testing-via-github-actions branch from bf0e39c to d734d43 Compare March 26, 2021 14:58
@myii myii merged commit fb72e66 into saltstack-formulas:master Mar 26, 2021
@myii myii deleted the ci/add-windows-testing-via-github-actions branch March 26, 2021 16:30
@myii
Copy link
Member Author

myii commented Mar 26, 2021

Thanks for the review, @dafyddj.

@saltstack-formulas-travis

🎉 This PR is included in version 0.13.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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